Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Display EntityValue labels in CSV export #2170

Merged
merged 3 commits into from
Mar 21, 2023
Merged

Conversation

gsingh93
Copy link
Contributor

@gsingh93 gsingh93 commented Mar 13, 2023

EntityValues are currently exported in the CSV as [Object object], this PR returns the label for the EntityValue instead, which is more useful.

Fixes #981

Checklist

  • CHANGELOG.md has been updated to incorporate all user visible changes made by this pull request.
  • Issues have been created for any UI or other user-facing changes made by this pull request.
  • [Maintainers only] If this pull request makes user-facing changes that require documentation changes, open a corresponding docs pull request in the github/codeql repo and add the ready-for-doc-review label there.

@shati-patel
Copy link
Contributor

👋🏽 Thanks for the PR, @gsingh93! I've just kicked of CI for now. Feel free to take this PR out of draft mode when you're ready for a proper review 😻

@gsingh93
Copy link
Contributor Author

Thanks! I think I just needed to get some time to update the CHANGELOG which I can do next week. I'm not sure exactly what's needed for this: "Issues have been created for any UI or other user-facing changes made by this pull request". Is this considered a user-facing change? Does this just mean that there is some issue like #981 that references it?

@shati-patel
Copy link
Contributor

shati-patel commented Mar 20, 2023

Thanks! I think I just needed to get some time to update the CHANGELOG which I can do next week.

Great, thanks!

I'm not sure exactly what's needed for this: "Issues have been created for any UI or other user-facing changes made by this pull request". Is this considered a user-facing change? Does this just mean that there is some issue like #981 that references it?

Yep, you're absolutely right! Feel free to check that box. It's a user-facing (albeit minor) change, and since there's an existing issue (#981) describing the problem, no need to create a new one 🎉
Thank you for being so thoughtful when contributing!

@gsingh93 gsingh93 marked this pull request as ready for review March 20, 2023 20:38
@gsingh93 gsingh93 requested a review from a team as a code owner March 20, 2023 20:38
Copy link
Contributor

@shati-patel shati-patel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good! Thank you again for the contribution ✨ Your change will be included in the next release (hopefully within the next few days 🤞🏽 )

extensions/ql-vscode/CHANGELOG.md Outdated Show resolved Hide resolved
@shati-patel shati-patel enabled auto-merge (squash) March 21, 2023 11:44
@shati-patel shati-patel merged commit bb0c53d into github:main Mar 21, 2023
@gsingh93 gsingh93 deleted the csv branch March 21, 2023 16:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Automatically convert CSV fields to strings
2 participants