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

Fix the url in the "build" badge in the readme #197

Merged
merged 2 commits into from
Mar 14, 2024

Conversation

jagerber48
Copy link
Contributor

@jagerber48 jagerber48 commented Mar 8, 2024

See #194 (comment)

Test the correct url here on my fork.

@newville
Copy link
Member

newville commented Mar 8, 2024

The URL works for me, but I think the target should be to show the success or failure of the test, not the test script itself. I would recommend a target of https://github.com/lmfit/uncertainties/actions/workflows/python-package.yml

Also: if you are editing "README.rst", perhaps the call for maintainers should be removed too ;)

@jagerber48
Copy link
Contributor Author

Ok, I pointed the url to the actions page instead of the action source code.

I also removed the call for maintainers. Should this go in another PR though? On this specific one though, I'd like to give @lebigot a chance to veto this change though since he put the message there in the first place and I feel like I'd be happy to know he's satisfied before removing the message.

:target: https://github.com/lmfit/uncertainties/blob/main/.github/workflows/python-package.yml

**Call for maintainers**: if you want this project to keep living and are ready to maintain it (pull requests management, issue resolution…), please contact me! I am ready to share my knowledge of the code logic by participating in discussions (notably around pull requests and issues).
:target: https://github.com/lmfit/uncertainties/actions/workflows/python-package.yml
Copy link
Member

Choose a reason for hiding this comment

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

Thanks!

@newville
Copy link
Member

newville commented Mar 9, 2024

@jagerber48 Thanks -- this looks good to me. If there are no objections, I'd be happy to merge within a couple of days.

@lebigot
Copy link
Collaborator

lebigot commented Mar 13, 2024

I also removed the call for maintainers. Should this go in another PR though? On this specific one though, I'd like to give @lebigot a chance to veto this change though since he put the message there in the first place and I feel like I'd be happy to know he's satisfied before removing the message.

Good with me! Based on my past interactions with all of you, I consider that the right people are now at the helm, and I trust that you'll welcome other people based on the contributions that they want to make!

@newville
Copy link
Member

@jagerber48 let's start by merging this one!

@newville newville merged commit ec15317 into lmfit:master Mar 14, 2024
16 checks passed
@jagerber48 jagerber48 deleted the bugfix/build_badge_url branch March 14, 2024 21:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants