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

Migrate Danger from GitHub Actions + ESP-IDF project #1

Merged
merged 10 commits into from
Dec 4, 2023

Conversation

tomassebestik
Copy link
Member

@tomassebestik tomassebestik commented Nov 28, 2023

Summary of changes

  • Framework Migration: The existing framework has been extracted from the espressif/github-actions repository.
  • Configurable Rule Parameters: Rule parameters can now be configured in the target workflow using the with: section.
  • Codebase Tests: Added comprehensive tests to cover the entire codebase.
  • Project Settings: As a separate project, we can now implement common project settings such as Prettier, ESLint, Jest, a Contributions Guide, and an issue tracker.
  • Documentation Overhaul: Updated documentation, with README now split into smaller files. We've added extensive documentation for each rule, similar to GitLab's Shared-CI-DangerJS.
  • New Rule - Source Branch Check: Introduced a rule for checking the correctness of the source branch name.
  • GitHub Action CI Job Trace Log: The trace log now includes a table with configuration parameters and the exit state of each individual rule, same as in GitLab's Shared-CI-DangerJS.

Updated rules

  • Description rule: Now have configurable ignored sections.
  • Commit Messages rule: Implements conventional commits, aligned with GitLab's Shared-CI-DangerJS and the default of Conventional-Precommit-Linter.
  • Number of Commits rule: Introduces a configurable threshold for warning/info type of Danger issue.
  • Source Branch rule: New in GH Danger - Ensures the source branch name is correct.

Output instructions

  • Review and Merge Process: The process is now dynamically presented (based on whether the GH project is a mirror) in a collapsible section.
  • Danger Instructions: The explanation of how Danger works and how to fix issues is now dynamic and presented in a collapsible section.
  • Extended Info for GitLab Mirrors: If the GitHub project is solely a GitLab mirror, additional information is provided about the review and merge process.
  • Contributors Guide Link: Option to enable a link to the project's Contributors Guide in the Danger comment.
  • Contributor License Agreement (CLA) Link: Option to include a link to the project's Contributor License Agreement in the Danger comment.

Styles

Tested

Related

  • RDT-601
  • RDT-573

Next steps

Copy link

github-actions bot commented Nov 28, 2023

Warnings
⚠️ Please consider squashing your 10 commits (simplifying branch history).
Messages
📖 This PR seems to be quite large (total lines of code: 2748), you might consider splitting it into smaller PRs

👋 Hello tomassebestik, we appreciate your contribution to this project!


📘 Please review the project's Contributions Guide for key guidelines on code, documentation, testing, and more.

Click to see more instructions ...


This automated output is generated by the PR linter DangerJS, which checks if your Pull Request meets the project's requirements and helps you fix potential issues.

DangerJS is triggered with each push event to a Pull Request and modify the contents of this comment.

Please consider the following:
- Danger mainly focuses on the PR structure and formatting and can't understand the meaning behind your code or changes.
- Danger is not a substitute for human code reviews; it's still important to request a code review from your colleagues.
- Resolve all warnings (⚠️ ) before requesting a review from human reviewers - they will appreciate it.
- Addressing info messages (📖) is strongly recommended; they're less critical but valuable.
- To manually retry these Danger checks, please navigate to the Actions tab and re-run last Danger workflow.

Review and merge process you can expect ...


We do welcome contributions in the form of bug reports, feature requests and pull requests.

1. An internal issue has been created for the PR, we assign it to the relevant engineer.
2. They review the PR and either approve it or ask you for changes or clarifications.
3. Once the GitHub PR is approved we do the final review, collect approvals from core owners and make sure all the automated tests are passing.
- At this point we may do some adjustments to the proposed change, or extend it by adding tests or documentation.
4. If the change is approved and passes the tests it is merged into the default branch.

Generated by 🚫 dangerJS against b595088

@tomassebestik tomassebestik force-pushed the add/danger-rules branch 29 times, most recently from 37cc581 to 0a1323f Compare December 1, 2023 07:46
@tomassebestik tomassebestik force-pushed the add/danger-rules branch 7 times, most recently from c3abe07 to e7a60c1 Compare December 1, 2023 10:43
@tomassebestik tomassebestik changed the title Develop Migrate Danger from GitHub Actions + ESP-IDF project Dec 1, 2023
@tomassebestik tomassebestik self-assigned this Dec 1, 2023
@tomassebestik tomassebestik added the Status: Reviewing Issue is being reviewed label Dec 1, 2023
@tomassebestik
Copy link
Member Author

@kumekay @dobairoland @alekseiapa PTAL ... it's very similar to Shared Danger Gitlab now. No need for in-depth code review logic, the code has full test coverage and has been tested in an external repo. Split to commits in logic sections. Many thanks, guys!

Copy link
Collaborator

@dobairoland dobairoland left a comment

Choose a reason for hiding this comment

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

LGTM in general. I didn't do a detailed review.

Copy link
Collaborator

@kumekay kumekay left a comment

Choose a reason for hiding this comment

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

@tomassebestik I made a relatively quick review, but nothing catches my eye, looks good, thank you

@tomassebestik tomassebestik merged commit 3507806 into v1 Dec 4, 2023
10 checks passed
@tomassebestik tomassebestik deleted the add/danger-rules branch December 4, 2023 17:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Reviewing Issue is being reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants