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

feat: README Badges and Execexam Logo #19

Closed
wants to merge 70 commits into from

Conversation

Chezka109
Copy link
Collaborator

@Chezka109 Chezka109 commented Sep 23, 2024

Pull Request: Add Dynamic Badges and Create Logo for execexam

Contributors:

Linked Issue:

Labels:

  • enhancement
  • infrastructure

Description:
This pull request introduces two key updates to the execexam project:

  1. Dynamic Badges:

    • Added a dynamic code coverage badge that updates its color based on the current test coverage percentage. This allows users to instantly see the coverage status at a glance.
    • Implemented a maintenance status badge to reflect the ongoing support and development of the project.
    • Future additions will include badges for the PyPI version and a passing build status, providing further transparency into the project state.
  2. Project Logo:

    • Created and added a new logo for execexam to improve project branding and visibility. The logo will be displayed prominently in the repository’s README file.

Coverage:

  • Test coverage has been maintained with these changes. The coverage badge confirms that the tests are running and coverage is accurately reported.

Testing:

  • This PR has been tested on:
    • Ubuntu 22.04 LTS via GitHub Actions
    • macOS Monterey
    • Windows 11
  • Tests were conducted using the project's continuous integration (CI) pipeline to ensure that badges are dynamically updated and the logo is properly displayed.

Screenshots:

  • Dynamic Coverage Badge:

    Coverage Badge

  • New Project Logo:

    execexam Logo

PCain02 and others added 30 commits September 5, 2024 15:33
…checking, python support, and dependency install
@Chezka109
Copy link
Collaborator Author

@PCain02, so for now the badges update when the command python badges.py is ran. But ideally, it would be in the build.yml file. The reason for this is because we're prioritizing other features rather than automating badges and is [for now] unnecessary when the manual process is working adequately. There could also be integration issues where automating badge updates may require significant changes to the existing CI/CD pipeline, which could introduce potential issues or delays in other processes.

@boulais01
Copy link
Collaborator

@PCain02 @Chezka109 Hello, do we think this this wait to be merged until after the version updating PR is complete(PR #18 ), or before? The version updating is waiting on a test from a Mac user.

@Chezka109
Copy link
Collaborator Author

@boulais01, I have just tested PR #18 on a Mac and can confirm it works. Since there is a version tag, it would make more sense for PR #18 to be complete. @PCain02, how does that sound to you? After it has been merged, I can update the version badge as well to reflect the updates.

@PCain02
Copy link
Collaborator

PCain02 commented Sep 23, 2024

Sounds good to me! I will say the version badge should be based on the actual version which I believe is 0.3.0 and not on the test version is which is like 0.3.39. So make sure it reflects the actual version number.

Copy link
Collaborator

@AlishChhetri AlishChhetri left a comment

Choose a reason for hiding this comment

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

The logo looks great, and I'm glad to see #e5e5e5 getting some love! Overall, this LGTM, but I would recommend waiting to merge this PR until the python badges.py command can be incorporated into GitHub Actions and fully automated. This will ensure that badges are updated seamlessly without manual intervention, aligning with the team's long-term goal of automating these processes.

Copy link
Collaborator

@gkapfham gkapfham left a comment

Choose a reason for hiding this comment

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

Hello @Chezka109, this PR is making strong improvements in a positive direction, thanks for creating it. However, we can't merge this PR if it creates the badges.py file in the main directory of the project. Generally, we avoid creating Python source code files in the main project directory that is reserved for configuration files and the README.md file. Can you please consider moving the aforementioned Python script to, for instance, a scripts directory?

Also, why is there a need for a separate ci.yml file. Why can't this go in the main build.yml file.

Finally, please note that this PR is likely going to be merged after the PR that sets up publishing to PyPI. You will need to make sure that you merge in all of those changes so that we don't delete/modify any configurations that have already been approved and merged into the main branch of the tool.

@Chezka109
Copy link
Collaborator Author

@gkapfham the changes have been made.

Copy link
Collaborator

@PCain02 PCain02 left a comment

Choose a reason for hiding this comment

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

Are we going to be closing this PR since we now have #24 from @gkapfham ? The main difference I see is test_util.py is not in #24. Let me know what the plan is going forward thanks!

@gkapfham
Copy link
Collaborator

Hello @Chezka109, if this PR is only about badges and the logo, then why are there so many commits not connected to these two elements? Also, please note that your build is failing and thus no members of our team are going to commit the time and resources to reviewing your PR.

@gkapfham
Copy link
Collaborator

Hello @Chezka109 this PR has a failing build. Do you plan to move forward with it or should we close it because you do not plan to resolve the build and move forward with this documentation feature?

@gkapfham gkapfham marked this pull request as draft October 25, 2024 19:19
@gkapfham
Copy link
Collaborator

Hello @Chezka109 can this PR be closed?

@Chezka109 Chezka109 closed this Oct 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request infrastructure CI/CD configuration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature: Add badges to ReadMe
7 participants