Skip to content

Update Danger setup to run via Buildkite + Add RuboCop linter #627

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

Merged
merged 6 commits into from
Feb 3, 2025

Conversation

mokagio
Copy link
Contributor

@mokagio mokagio commented Feb 2, 2025

What does it do?

In #410, Danger failed to run because of an issue installing native extensions. This made @iangmaia and I realize that Dangermattic in this repo was not set up with the modern Buildkite configuration, but still run on GitHub Actions.

This PR updates the Danger step up.

image

image

I used WordPress at d7cc886 at the template.

While I was at it, I also added a CI step to run the RuboCop linter.

image

Checklist before requesting a review

  • Run bundle exec rubocop to test for code style violations and recommendations. — N.A.
  • Add Unit Tests (aka specs/*_spec.rb) if applicable. — N.A.
  • Run bundle exec rspec to run the whole test suite and ensure all your tests pass. — N.A.
  • Make sure you added an entry in the CHANGELOG.md file to describe your changes under the appropriate existing ### subsection of the existing ## Trunk section. — N.A.
  • If applicable, add an entry in the MIGRATION.md file to describe how the changes will affect the migration from the previous major version and what the clients will need to change and consider. — N.A.

@dangermattic
Copy link
Collaborator

dangermattic commented Feb 2, 2025

1 Warning
⚠️ Please add an entry in the CHANGELOG.md file to describe the changes made by this PR

Generated by 🚫 Danger

@mokagio mokagio marked this pull request as ready for review February 2, 2025 23:08

- group: "Linters"
steps:
- label: ☢️ Danger - PR Check
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I updated the branch requirements to use this name.

As a follow up, I shall open a new PR implementing the custom GitHub commit status names.

secrets:
github-token: ${{ secrets.DANGERMATTIC_GITHUB_TOKEN }}
buildkite-api-token: ${{ secrets.TRIGGER_BK_BUILD_TOKEN }}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Notice that this workflow didn't kick in for this PR, as you can see by the fact that the Dangermattic comment still mentions the PR being a draft. But that's because for it to run it needs to land on trunk.

Copy link
Contributor

Choose a reason for hiding this comment

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

🎗️ To add TRIGGER_BK_BUILD_TOKEN to the GHA secrets in this repo.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. Thanks for the reminder!

image

Copy link
Contributor

@iangmaia iangmaia left a comment

Choose a reason for hiding this comment

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

Approving to unblock, but left a couple of improvements / suggestions.


on:
pull_request:
types: [opened, reopened, ready_for_review, synchronize, edited, review_requested, review_request_removed, labeled, unlabeled, milestoned, demilestoned]
types: [labeled, unlabeled, milestoned, demilestoned]
Copy link
Contributor

Choose a reason for hiding this comment

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

By the way, just remembered that given we now don't trigger Buildkite when a PR moves to ready for review, it probably makes sense to also add the ready_for_review event as well.

mokagio and others added 2 commits February 4, 2025 07:45
Co-authored-by: Ian G. Maia <iangmaia@users.noreply.github.com>
Co-authored-by: Ian G. Maia <iangmaia@users.noreply.github.com>
@mokagio mokagio merged commit 26b294a into trunk Feb 3, 2025
6 checks passed
@mokagio mokagio deleted the mokagio/danger-buildkite branch February 3, 2025 22:25
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.

4 participants