-
-
Notifications
You must be signed in to change notification settings - Fork 347
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
checks: Add display and other modules root folder as accepted title prefixes #5341
base: main
Are you sure you want to change the base?
Conversation
That's inconsistent, isn't it? |
I think it was originally used when "raster" or "vector" work was for the C library, not modules. That's my interpretation. So I didn't want to blindly change the interpretation/classification of older PR titles in 8.5 |
The purpose is to help sort out release notes whichare meant for users (of any kind). Linting fixes should probably be checks or qc rather than library or modules. |
@wenzeslaus Can you look at #5345 for what you would like as a title in regards to this? Otherwise that PR is already ready |
I used "tools: Fix typos in imagery tools" for #5345. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm against "embracing" the inconsistency (temporal here vs raster and vector for library).
What do you suggest? |
I don't have an idea beyond the title I used which is within the current rules. |
But then again, the motivation behind this was to auto-organize release notes. This can be changed at any point plus the code is already custom code, so it is probably possible to invalidate some titles while still sorting them. |
As we can see in #5328, the title "display: fix typos" doesn't fit in our classification regex. I think it is a valid title as it groups changes from different d.* modules, in the display folder.
So, I added that folder, plus other folders containing modules as valid prefixes.
Note that I didn't include "raster" or "vector", as they are already part of another section below, and I didn't check how it would behave, so I'm keeping it safe.