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

[GitHub] add latest release of a branch badge #9661

Conversation

karolwydmuch
Copy link

No description provided.

@github-actions
Copy link
Contributor

Messages
📖 ✨ Thanks for your contribution to Shields, @karolwydmuch!

Generated by 🚫 dangerJS against a63bc37

@chris48s
Copy link
Member

Hi. Thanks for opening a PR.

A badge should be quick to render. In most contexts where our badges are displayed there's a hard limit of 4 seconds on a request. In general we try usually avoid accepting badges that require more than one network call to render for this reason.

This means badges that require iterating over multiple pages of API response are impractical. They end up being too slow to render in a lot of cases.

I had a look to see if it would be possible to obtain this information in a single GraphQL query but it seems this is not exposed by the GraphQL API. https://docs.github.com/en/graphql/reference/objects#release

If we can't find a more efficient way to query it, I'm inclined to decline this one.

Out of interest, and also for testing purposes, do you have an example of a repo that publishes releases against different branches? It is not something I have come across before and I don't think this corresponds to an open feature request.

})

t.create('Release')
.get('/v/release/expressjs/express.json')
Copy link
Member

Choose a reason for hiding this comment

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

If we end up coming back to this, none of the tests in this file are actually testing the service class that has been added in this PR, but lets not get bogged down in that for the moment

@PyvesB
Copy link
Member

PyvesB commented Jun 1, 2024

@karolwydmuch any thoughts following Chris's comments? :)

@PyvesB
Copy link
Member

PyvesB commented Jul 3, 2024

Given that there hasn't been any activity for a while, I'm going to close this pull request. Feel free to ping us or reopen it if you get a chance to look into it again. 😉

If others think that repository level projects would be an option and want to pick this up, you can simply fetch the commits by running git fetch origin pull/9661/head:pr-9661. Change the implementation to target repository level projects and open a new pull request with a co-authored trailer included in the commit message to give attribution to the original author.

@PyvesB PyvesB closed this Jul 3, 2024
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.

None yet

4 participants