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

Add [galaxytoolshed] services #8114

Merged
merged 5 commits into from
Jul 2, 2022
Merged

Add [galaxytoolshed] services #8114

merged 5 commits into from
Jul 2, 2022

Conversation

guillaume-gricourt
Copy link
Contributor

@guillaume-gricourt guillaume-gricourt commented Jun 23, 2022

This PR refers to the PR #7633 and the issue #7660 suggesting one feature, get the number of downloads for a repository, for a new service galaxytoolshed.
Many thanks

@shields-ci
Copy link

shields-ci commented Jun 23, 2022

Messages
📖 ✨ Thanks for your contribution to Shields, @guillaume-gricourt!

Generated by 🚫 dangerJS against 261c942

Copy link
Member

@calebcartwright calebcartwright left a comment

Choose a reason for hiding this comment

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

Thank you for your patience, persistence, and breaking up the original PR as this is much easier to review and discuss!

I think the bones of this are in place, but there's a few changes I'd like to see and a couple questions I've left inline below

services/galaxytoolshed/galaxy-toolshed-base.js Outdated Show resolved Hide resolved
services/galaxytoolshed/galaxy-toolshed-base.js Outdated Show resolved Hide resolved
services/galaxytoolshed/galaxy-toolshed-base.js Outdated Show resolved Hide resolved
services/galaxytoolshed/galaxy-toolshed-base.js Outdated Show resolved Hide resolved
@calebcartwright calebcartwright added the service-badge New or updated service badge label Jun 25, 2022
@calebcartwright calebcartwright changed the title Add [galaxytoolshed] services Add galaxytoolshed services Jun 28, 2022
@calebcartwright
Copy link
Member

Puzzled by the CI failure so changing the title to test some things

@calebcartwright calebcartwright changed the title Add galaxytoolshed services Add [galaxytoolshed] services Jun 29, 2022
@calebcartwright
Copy link
Member

CI should be fixed. Can you rebase your branch?

Copy link
Member

@calebcartwright calebcartwright left a comment

Choose a reason for hiding this comment

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

Couple minor cleanup items, but happy to approve and merge after they are taken care of!

services/galaxytoolshed/galaxytoolshed-base.js Outdated Show resolved Hide resolved
@calebcartwright
Copy link
Member

It's going to be tricky getting this merged because it's coming from an Organization-owned fork. We have a branch protection policy in place that requires the source branch be up to date with the target before PRs can be merged.

We utilize bots and automation which make this seamless for most contributors, but GitHub doesn't allow PRs from Org forks to enable write access to the upstream maintainers (and their bots). This means we'll likely have to try to coordinate a squash/merge update and approval to happen at the same time, as any time any other PR is merged in this repo it'll reset yours back to an unmergable, out-of-date state

@guillaume-gricourt
Copy link
Contributor Author

If I open a new PR, the same as this PR, with a fork coming from my personal account, can be helpful ?

@calebcartwright
Copy link
Member

If you can go ahead and update this one shortly then we can probably get it resolved. For future PRs though yes it would definitely be worth considering submitting them from a personal fork

@calebcartwright calebcartwright merged commit 7d088eb into badges:master Jul 2, 2022
@guillaume-gricourt
Copy link
Contributor Author

Thank you so much !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
service-badge New or updated service badge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants