Skip to content

Starcke/qlpack gen rename#2099

Merged
starcke merged 2 commits intomainfrom
starcke/qlpack-gen-rename
Feb 21, 2023
Merged

Starcke/qlpack gen rename#2099
starcke merged 2 commits intomainfrom
starcke/qlpack-gen-rename

Conversation

@starcke
Copy link
Copy Markdown
Contributor

@starcke starcke commented Feb 21, 2023

The first commit changes the name of the generated pack from qlpack.yml to codeql-pack.yml.

The second commit is a single error message change and some comment changes. I noticed them when doing the change, but I can just revert that commit if we do not want to change it.

Checklist

N/A.

  • 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.

@starcke starcke requested review from a team as code owners February 21, 2023 08:52
@norascheuch
Copy link
Copy Markdown
Contributor

Regarding the changed comments: do we want to advertise both names?

@starcke
Copy link
Copy Markdown
Contributor Author

starcke commented Feb 21, 2023

Regarding the changed comments: do we want to advertise both names?

Not sure. I am not super familiar with the context for the change, but I found it odd to refer to a specific name when it could actually be another name. An alternative is to use a less concrete term like "codeql pack file" but often a concrete name is easier to understand.

Copy link
Copy Markdown
Contributor

@norascheuch norascheuch left a comment

Choose a reason for hiding this comment

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

Elena confirmed since both names are possible we can advertise them!

Copy link
Copy Markdown
Contributor

@elenatanasoiu elenatanasoiu left a comment

Choose a reason for hiding this comment

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

We support both qlpack.yml and codeql-pack.yml at the moment (see https://github.com/github/vscode-codeql/pull/2054/files). So I think your change is perfect @starcke 👍 Thanks for doing it!

@starcke starcke merged commit 7b4b491 into main Feb 21, 2023
@starcke starcke deleted the starcke/qlpack-gen-rename branch February 21, 2023 15:34
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.

3 participants