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(github-actions): create a unified status action #912

Closed

Conversation

josephperrott
Copy link
Member

Create a github action which tracks all of the statuses on a pull request to create a unified status value.

@josephperrott josephperrott added the action: review The PR is still awaiting reviews from at least one requested reviewer label Nov 28, 2022
@angular-robot angular-robot bot added the detected: feature PR contains a feature commit label Nov 28, 2022
Create a github action which tracks all of the statuses on a pull request to create a unified status value.
@josephperrott josephperrott self-assigned this Nov 28, 2022
]);

/** GraphQL schema for requesting the status information for a given pull request. */
const PR_SCHEMA = params(
Copy link
Member

Choose a reason for hiding this comment

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

isn't all of this similar in ng-dev? can we just use that from there or dedupe?

Copy link
Member Author

Choose a reason for hiding this comment

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

We could, but to do so we would have to use the AuthenticatedGithubApi class that is in ng-dev, which is more complicated because of the access to the configuration files.

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

I was thinking of the AuthenticatedGitClient rather than the Github one. Will see what I can do.

Copy link
Member Author

Choose a reason for hiding this comment

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

From looking into it more, we can leverage the ng-dev stuff, but we don't really save ourselves that much code since we still have to describe what the shape of the PR response we want is.

Copy link
Member

Choose a reason for hiding this comment

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

I thought we are already using a similar schema (almost identical, maybe with a bit more stuff). If that's not the case, then yes. I'm fine keeping it here. Maybe want to extract it into a separate file for more readability? it's odd having this at the end of a file too (not relying on hoisting for better readability)

Copy link
Member Author

Choose a reason for hiding this comment

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

I will do the refactor into another file in a follow up because I want to expand a bit of what its doing and I would rather land this as is for now.

Copy link
Member

Choose a reason for hiding this comment

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

no worries, sgtm

Copy link
Member

@devversion devversion left a comment

Choose a reason for hiding this comment

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

LGTM. some minor suggestions, but nice work!

]);

/** GraphQL schema for requesting the status information for a given pull request. */
const PR_SCHEMA = params(
Copy link
Member

Choose a reason for hiding this comment

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

I thought we are already using a similar schema (almost identical, maybe with a bit more stuff). If that's not the case, then yes. I'm fine keeping it here. Maybe want to extract it into a separate file for more readability? it's odd having this at the end of a file too (not relying on hoisting for better readability)

@josephperrott josephperrott added action: merge The PR is ready for merge by the caretaker and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Nov 29, 2022
@angular-robot
Copy link
Contributor

angular-robot bot commented Nov 29, 2022

This PR was merged into the repository by commit 856c3ab.

@angular-robot angular-robot bot closed this in 856c3ab Nov 29, 2022
@josephperrott josephperrott deleted the unified-status-check branch November 30, 2022 19:08
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Dec 31, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker detected: feature PR contains a feature commit
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants