-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Rename CI job names #5618
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
Rename CI job names #5618
Conversation
Pull Request Test Coverage Report for Build 1640380610
π - Coveralls |
We probably need to change some setting about which actions are required for PRs to mergeable for this to work. |
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.
Tbh at first glance I though this looked quite good, but the CI test summery is much more readable with the old style. Could be the multiple slashes that make it more difficult to read.
I would suggest to keep the old style.
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.
LGTM ! Some suggestions
Co-authored-by: Pierre Sassoulas <pierre.sassoulas@gmail.com>
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.
LGTM, but I think it requires that I change the settings of the repositories for required jobs, so it might be a mess during the time where both old job exists on out of date branch and new jobs on up to date branch. Maybe it's time to remove required job because all maintainers are responsible individual that will only merge responsibly anyway ? (Also if Marc think the old one are better and we still require some job to pass, it would creates work for an opinionated change and we may choose to close)
I think that should be fine. That had been the default a while back anyway.
Just my opinion, in the PR view I definitely think the old style looked better. The only advantage I see is in the actions side panel, but just ever so slightly. I mentioned it earlier already, it could be the multiple slashes for each entry which make it much more difficult to read. Overall I'm somewhere between -0.5 and -1 on this one. However if you both think it's a definite improvement, I guess I could live with it too. |
I removed the required job and I think this is making the test name easier to parse by giving predictability. As this is not a very important change let's go with the majority opinion here. |
Co-authored-by: Pierre Sassoulas <pierre.sassoulas@gmail.com>
Type of Changes
Description
Taking inspiration from
pip
as seen here pypa/pip#10720 I really like their clean CI jobs names.This is probably personal preference, but would we consider changing them? This is my suggestion based on their design.