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

[libc++][CI] Moves CI badge to main README. #77247

Merged
merged 1 commit into from
Jan 9, 2024
Merged

Conversation

mordante
Copy link
Member

@mordante mordante commented Jan 7, 2024

The current CI badge is currently in libc++ documentation. This does not seem the right place:

  • The typical location on GitHub is on the main README.
  • The documentation is shipped as part of the release:
    • This link does not work in off-line mode. Currently our documentation works in off-line mode.
    • The status in the release documentation does not reflect the status of the shipped library. So users looking at it may see a red status and get confused.

This moves the badge to the README.

The current CI badge is currently in libc++ documentation. This does not
seem the right place:
- The typical location on GitHub is on the main README.
- The documentation is shipped as part of the release:
  - This link does not work in off-line mode. Currently our documentation
    works in off-line mode.
  - The status in the release documentation does not reflect the status of
    the shipped library. So users looking at it may see a red status and
    get confused.

This moves the badge to the README.
@mordante mordante requested a review from a team as a code owner January 7, 2024 18:59
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Jan 7, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Jan 7, 2024

@llvm/pr-subscribers-libcxx

Author: Mark de Wever (mordante)

Changes

The current CI badge is currently in libc++ documentation. This does not seem the right place:

  • The typical location on GitHub is on the main README.
  • The documentation is shipped as part of the release:
    • This link does not work in off-line mode. Currently our documentation works in off-line mode.
    • The status in the release documentation does not reflect the status of the shipped library. So users looking at it may see a red status and get confused.

This moves the badge to the README.


Full diff: https://github.com/llvm/llvm-project/pull/77247.diff

2 Files Affected:

  • (modified) README.md (+1)
  • (modified) libcxx/docs/index.rst (-4)
diff --git a/README.md b/README.md
index 4ae7eaf9b083a5..8202ff61d42fb9 100644
--- a/README.md
+++ b/README.md
@@ -1,6 +1,7 @@
 # The LLVM Compiler Infrastructure
 
 [![OpenSSF Scorecard](https://api.securityscorecards.dev/projects/github.com/llvm/llvm-project/badge)](https://securityscorecards.dev/viewer/?uri=github.com/llvm/llvm-project)
+[![libc++](https://github.com/llvm/llvm-project/actions/workflows/libcxx-build-and-test.yaml/badge.svg?branch=main&event=schedule)](https://github.com/llvm/llvm-project/actions/workflows/libcxx-build-and-test.yaml?query=event%3Aschedule)
 
 Welcome to the LLVM project!
 
diff --git a/libcxx/docs/index.rst b/libcxx/docs/index.rst
index c7769bae6bb17d..bcaf99495b8fac 100644
--- a/libcxx/docs/index.rst
+++ b/libcxx/docs/index.rst
@@ -202,10 +202,6 @@ Design Documents
 Build Bots and Test Coverage
 ============================
 
-.. image:: https://github.com/llvm/llvm-project/actions/workflows/libcxx-build-and-test.yaml/badge.svg?branch=main&event=schedule
-   :target: https://github.com/llvm/llvm-project/actions/workflows/libcxx-build-and-test.yaml?query=event%3Aschedule
-   :alt: Build and Test libc++
-
 * `Github Actions CI pipeline <https://github.com/llvm/llvm-project/actions/workflows/libcxx-build-and-test.yaml>`_
 * `Buildkite CI pipeline <https://buildkite.com/llvm-project/libcxx-ci>`_
 * `LLVM Buildbot Builders <https://lab.llvm.org/buildbot>`_

Copy link
Member

@ldionne ldionne left a comment

Choose a reason for hiding this comment

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

This makes sense to me -- I only wonder if we want to put this in a less prominent location since this is the README for the whole LLVM project, and the libc++ CI status is only relevant for a small portion of the project.

But I totally agree that it doesn't belong in the documentation -- I think I had raised that concern in the original patch that added it and I appreciate that you took it on yourself to fix the issue!

@mordante
Copy link
Member Author

mordante commented Jan 8, 2024

This makes sense to me -- I only wonder if we want to put this in a less prominent location since this is the README for the whole LLVM project, and the libc++ CI status is only relevant for a small portion of the project.

I checked and libc++ is currently the only scheduled CI job. I hope other subprojects will add their badges when they do so. I added @tstellar as reviewer since he reviewed the other badge we have.

Copy link
Collaborator

@tstellar tstellar left a comment

Choose a reason for hiding this comment

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

This is fine with me. If people don't like it we can always move it somewhere else.

Copy link
Member

@ldionne ldionne left a comment

Choose a reason for hiding this comment

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

Ok, great, fine with me as well then.

@mordante
Copy link
Member Author

mordante commented Jan 9, 2024

Thanks for the reviews!

@mordante mordante merged commit f0fd8fd into llvm:main Jan 9, 2024
47 checks passed
@mordante mordante deleted the ci_badge branch January 9, 2024 18:12
justinfargnoli pushed a commit to justinfargnoli/llvm-project that referenced this pull request Jan 28, 2024
The current CI badge is currently in libc++ documentation. This does not
seem the right place:
- The typical location on GitHub is on the main README.
- The documentation is shipped as part of the release:
- This link does not work in off-line mode. Currently our documentation
works in off-line mode.
- The status in the release documentation does not reflect the status of
the shipped library. So users looking at it may see a red status and get
confused.

This moves the badge to the README.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants