-
Notifications
You must be signed in to change notification settings - Fork 186
docs: edited icons for workflows #1897
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
Conversation
merks
left a comment
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.
laeubi
left a comment
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.
|
Beside that I'm not sure if codefreeze period check makes any sense here... |
|
I can also add an old shield if that's important? |
|
I agree that the tests badge should be there. It's explicitly generated and deployed by the workflow publishing test results: eclipse.platform.swt/.github/workflows/junit.yml Lines 73 to 79 in 7825d59
|
|
I think that would be correct. |
|
Hey. You accept it |
|
As mentioned before (#1897 (comment)), in my opinion the tests badge should not be removed: (https://gist.githubusercontent.com/eclipse-releng-bot/78d110a601baa4ef777ccb472f584038/raw/71510599eb84e852f3e135aa7a3ddf33854ca716/badge.svg |
I fixed everything, check it again. |
|
This is what it would currently look like: I agree with @laeubi that the freeze period check does not make sense here. Reason is that the badges show results from latest master build and in freeze period we will usually not merge into master, so this badge will always show "passed" unless someone merged within freeze period. And then it would even report "failed" until the first PR is merged after freeze period. So only change that would remain with this PR is the addition of the badge for publishing test results. I think that one makes sense to add, as it indicates whether the badge containing the actual tests results may be out-of-date (as that one will not be updated in case the test results publish failed). If not one objects on adding that one, @braveocheretovych could you please remove the freeze period badge and squash your commits so that we may merge this? |
|
So I edited what you asked for, the only thing I don't know is how to compress it all into one commit. Maybe I should close this one and create a new one, what do you think? |
You don't need to create a new PR. In Git, you can squash commits via interactive rebase (or some GUI tools also have a dedicated "squash" functionality). You can find several tutorial on how to do it in the web, e.g., https://www.datacamp.com/tutorial/git-squash-commits |
|
I'm sorry to admit it, but I can't do this because I'm making the change through the edit tab on Github, I don't use the terminal to work. |
58336ba to
232b9f4
Compare
|
Alright, I've squashed the changes for you. |
|
Thankyou @HeikoKlare |
|
Do I need to take any further action? |
|
No, you don't need to take any further action. I just wanted to keep this open for a short while to give the chance to raise objections (see #1897 (comment)). |
|
I understand, thanks for the clarification. |
As part of working on eclipse-platform#2764 I was considering how to display badges going forward to align with test results per platform. As it turns out the "SWT Matrix Tests" has been stuck on an arbitrary old badge for many years now. This PR updates it, but as it has been wrong for a long time, perhaps removing it entirely makes more sense, but for now I update it. There was push back in eclipse-platform#1897 against removing it. Part of eclipse-platform#2714
The readme.md (before this change, but preserved with this PR) has it hardcoded to a specific old (3 year old) version. That 3 year old badge was from a day the tests failed, so the readme has shown failing badge for 3 years. See PR #2778 for an update. |
As part of working on #2764 I was considering how to display badges going forward to align with test results per platform. As it turns out the "SWT Matrix Tests" has been stuck on an arbitrary old badge for many years now. This PR updates it, but as it has been wrong for a long time, perhaps removing it entirely makes more sense, but for now I update it. There was push back in #1897 against removing it. Part of #2714



No description provided.