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

Unify pipeline status icons #4414

Merged
merged 5 commits into from
Nov 22, 2024
Merged

Unify pipeline status icons #4414

merged 5 commits into from
Nov 22, 2024

Conversation

xoxys
Copy link
Member

@xoxys xoxys commented Nov 19, 2024

I know this might be controversial and there were already a lot of back and forth by the pipeline status icons. But right now it is kind of inconsistent, e.g. status success, running and skipped are circled icons while others are not. I would really like to harmonize it and use circleOutline icons for all pipeline status icons.

image

@xoxys xoxys requested a review from a team November 19, 2024 09:34
@anbraten
Copy link
Member

Why are we migrating to the circle icons anyways we had: #2783 🤔

@xoxys
Copy link
Member Author

xoxys commented Nov 19, 2024

Right now it's inconsistent and to be honest, I don't even understand why we removed the circled versions in the linked PR. The plain icons look ugly and the different shapes make them look messy and not uniform in size for the eye.

@pat-s
Copy link
Contributor

pat-s commented Nov 19, 2024

👋️ hey guys, just trying to help. How about doing polls among maintainers on such subjective UI things? This would eliminate the 1:1 frictions and allows us to have more of a democratic way of moving forward.

@xoxys
Copy link
Member Author

xoxys commented Nov 19, 2024

Polls are fine for me, but often hard to get a majority as some maintainers are not participating or only read messages infrequently.

@anbraten
Copy link
Member

The reason we changed it before was that there are circle versions missing for some icons and therefore we sometimes mixed circle and none circle icons which we wanted to avoid. Seems like #3809 changed them unrelated to the plugin migration. Can't see a reason for any new discussions 🙃

@xoxys
Copy link
Member Author

xoxys commented Nov 19, 2024

For the status, not a single icon I can see is missing a circleOutline version.

Can't see a reason for any new discussions 🙃

There was never a discussion. You have not explained your reasons on the linked PR and feedback on this PR was ignored 🤷 However, feel free to close this PR... I feel a bit disappointed now as I have a feeling that efforts put into UI got blocked quite often...

@anbraten
Copy link
Member

There was never a discussion.

We had it in another PR or chat (not interested in searching the comment / message atm 😅).

The service gear would stay without a circle?

Screenshot from 2024-11-19 11-26-13

However, feel free to close this PR... I feel a bit disappointed now as I have a feeling that efforts put into UI got blocked quite often...

Please know that review comments are meant to improve the final outcome and ensure we’ve considered all perspectives.

@pat-s
Copy link
Contributor

pat-s commented Nov 19, 2024

Polls are fine for me, but often hard to get a majority as some maintainers are not participating or only read messages infrequently.

I see the issue but this is similar to the other governance-related issue we had: if we just continue to merge such PRs which are actively debated or contain subjective parts like UI changes with one maintainer approval, we might make the majority unhappy or force even a revert with the argument "I didn't see it and couldn't even veto it". In addition, this causes friction among maintainers and eventually leads to "everyone quickly merges subjective changes to not even allow much of a discussion".

The same applies to opinions of owners IMO, i.e. they shouldn't be a hard veto for such changes if "enough" people liked it/agreed on it.

All the above is without any opinion on the changes of this PR!

So we need to find a way to

  • allow all maintainers to cast a vote in a reasonable time
  • decide what the (majority) count should be that is required for merge (doesn't necessarily need to be > 50% of possible votes IMO)

(Yes I know this is off-topic and should be moved to a discussion but there it will likely go stale (?)).

Currently, the maintainers team consists of 8 real people of which 6 are reasonably active in such discussions.

As we can't enforce votes in timespan X from all members, I think we need to work with relative votes. In addition, the tagging of the maintainers team is used inflationary and most don't even look at the issues therefore.
Hence I propose the following:

  • No maintainer group tags anymore by default in PRs
  • Every UI-related change is coupled by a comment requesting to vote on the changes and tags the maintainers team (thumbs up/down)
  • The vote is kept open for three days and if there's a majority in favor of the changes, they get merged, otherwise not.
  • Only maintainer votes are counted, other user votes are discarded

Sorry for the wall of text. Yet, I think it is needed to find to reduce anger/emotions in decision making (I'll include myself here as well) and improves governance overall. @xoxys @anbraten what do you think?

@xoxys
Copy link
Member Author

xoxys commented Nov 19, 2024

We had it in another PR or chat (not interested in searching the comment / message atm 😅).

Just adding context to the PR description could solve it (we should improve in this area anyway)

The service gear would stay without a circle?

The gear is already a circle shape, which fits optically for me. Technically, I get your point, and we could search for another circleOutline icon (IMO a gear is not really symbolic for services anyway)

Please know that review comments are meant to improve the final outcome and ensure we’ve considered all perspectives.

You're right, sorry for my reaction, I'm trying to improve here.

@6543
Copy link
Member

6543 commented Nov 19, 2024

My criteria are just: contrast between them is high enouth; symboles can be spotted as different one evenif monocrome; the bird for running pipelines stays :)

So a vote is fine by me, i will just intentionaly abstain to let people witj better UI/UX experience deside it :)

@xoxys
Copy link
Member Author

xoxys commented Nov 19, 2024

@woodpecker-ci/maintainers please vote on this post: 👍 use circled pipeline icons 👎 use not circled icons from #2783

@pat-s
Copy link
Contributor

pat-s commented Nov 21, 2024

So, three in favor vs 2 abstaining (?). Guess that's a veto in favor then, as we can't get to a count of 3 against it anymore. Will merge later today, unless somebody else intervenes.

Copy link
Member

@6543 6543 left a comment

Choose a reason for hiding this comment

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

As per vote

@xoxys xoxys added ui frontend related enhancement improve existing features labels Nov 22, 2024
@xoxys xoxys merged commit 068893d into main Nov 22, 2024
7 checks passed
@xoxys xoxys deleted the unify-icons branch November 22, 2024 09:14
@woodpecker-bot
Copy link
Collaborator

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement improve existing features ui frontend related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants