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

docs: add release badge and LinkedIn badge #12531

Merged
merged 3 commits into from
Feb 1, 2024
Merged

Conversation

terrytangyuan
Copy link
Member

The categorization is to be consistent with Argo CD. Also added release badge and LinkedIn badge.

@terrytangyuan
Copy link
Member Author

Is there a way to use the root level README file in the docs website? e.g. https://github.com/argoproj/argo-workflows/blob/main/docs/README.md is pretty redundant and we don't want to update both places every time.

Copy link

@agilgur5 agilgur5 left a comment

Choose a reason for hiding this comment

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

I actually don't like the way Argo CD has badges with a lot of spacing and explicit categories, because that kind of defeats the purpose of badges as being small, aesthetically pleasing, in-line links.

Screenshot 2024-01-19 at 2 53 35 PM

If we wanted to do something similar, I would at least remove the double-spaced new-lines as suggested below, but I still think this is not quite how badges are intended to be used.

It also violates markdownlint & markdown conventions as well

@agilgur5 agilgur5 added the area/docs Incorrect, missing, or mistakes in docs label Jan 19, 2024
@@ -1,11 +1,18 @@
<!-- markdownlint-disable-next-line MD041 -->

Choose a reason for hiding this comment

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

this ignore is here because the first line is supposed to be an H1. If there's a line of badges first, I think that's ok, but putting several lines of text + badges would be an anti-pattern and would fail markdownlint multiple times. By convention, any text should be inside of / below headings

@agilgur5
Copy link

Is there a way to use the root level README file in the docs website? e.g. https://github.com/argoproj/argo-workflows/blob/main/docs/README.md is pretty redundant and we don't want to update both places every time.

If we want them to be equivalent, yes, I'm pretty sure there's a few ways to do it. They weren't the same until your #11358 I think, so that is a more recent change. I can handle that

Signed-off-by: Yuan Tang <[email protected]>
@terrytangyuan
Copy link
Member Author

Addressed comments

Signed-off-by: Yuan Tang <[email protected]>
@terrytangyuan terrytangyuan changed the title docs: Additional badges and categorization docs: Additional badges Jan 30, 2024
@terrytangyuan
Copy link
Member Author

Agreed that this looks ugly. I re-purposed the PR to just add the additional badges.

@terrytangyuan terrytangyuan merged commit 6a39edf into main Feb 1, 2024
16 checks passed
@terrytangyuan terrytangyuan deleted the terrytangyuan-patch-1 branch February 1, 2024 04:16
isubasinghe pushed a commit to isubasinghe/argo-workflows that referenced this pull request Feb 4, 2024
@agilgur5
Copy link

agilgur5 commented Mar 6, 2024

If we want them to be equivalent, yes, I'm pretty sure there's a few ways to do it. They weren't the same until your #11358 I think, so that is a more recent change. I can handle that

I added a script for this in #12752. It's not quite a straight copy+paste because of needing to change relative and absolute links.

agilgur5 pushed a commit that referenced this pull request May 4, 2024
Signed-off-by: Yuan Tang <[email protected]>
(cherry picked from commit 6a39edf)
@argoproj argoproj locked as resolved and limited conversation to collaborators Aug 28, 2024
@agilgur5 agilgur5 changed the title docs: Additional badges docs: add release badge and LinkedIn badge Aug 28, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area/docs Incorrect, missing, or mistakes in docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants