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: replaces 'check_only' with 'dry_run' option #195

Merged
merged 16 commits into from
Apr 19, 2024

Conversation

jpower432
Copy link
Contributor

@jpower432 jpower432 commented Mar 22, 2024

Description

Adds a dry run option, and sets output based on the information. Add the BotResults data class to make results easier to process as an input and output to multiple classes. Adds basic results reporting to make sure that the output is readable.

Related changes are in scope of the issue, but in different commits.

  • Add dry run option/remove check only
  • Add reporting with the additional results
  • Adds CI specific reporting for easier to read output (Changes can end up being a lot of information and it would need to collapse by default to ensure the logging output is still clear)

Fixes #181

Note: The release notes will require steps on how to migrate from check_only to dry_run.

Rationale

Failure upon diff doesn't make sense outside of the scope of auto-sync. This removes that and shifts the decision to fail to the user.

Use cases

  • Receiving change from forks that should not interact with the remote repository
  • Validation that the changes pass requirements before transformation on push

Type of change

  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How has this been tested?

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

@jpower432 jpower432 mentioned this pull request Mar 23, 2024
10 tasks
@jpower432 jpower432 force-pushed the feat/replace-check-only branch from e5dcf67 to 24d5ac9 Compare March 25, 2024 16:34
@jpower432 jpower432 added enhancement New feature or request breaking_change Backward incompatible changes labels Mar 25, 2024
@jpower432 jpower432 added this to the 0.9.0 milestone Mar 26, 2024
@jpower432 jpower432 marked this pull request as ready for review March 27, 2024 18:39
@jpower432 jpower432 marked this pull request as draft March 27, 2024 19:12
@jpower432 jpower432 removed this from the 0.9.0 milestone Mar 27, 2024
BREAKING CHANGE: The check_only flag is no longer available
Add more granular tests for dry run and reduces duplication

Signed-off-by: Jennifer Power <[email protected]>
Abstracts reporting results to the console to different class
and out of the entrypoint base class to allow it to extend more easily
and be used in different places in the code base.

Signed-off-by: Jennifer Power <[email protected]>
GitHub Actions and GitLab CI has specific formatting features that
can be used to enhance the output. Also the GitHub Actions output setting
has been moved to this report to remove the need to parse logs in bash

Signed-off-by: Jennifer Power <[email protected]>
This also refines the function through the TDD process

Signed-off-by: Jennifer Power <[email protected]>
@jpower432 jpower432 force-pushed the feat/replace-check-only branch from 392fd37 to bf72326 Compare March 29, 2024 23:30
For more comprehensiveness, adding local commit and not
push to remote in dry run feature and added change type for
more detailed change reports.

Signed-off-by: Jennifer Power <[email protected]>
This can be used in the release notes to show users how to migrate
from check_only

Signed-off-by: Jennifer Power <[email protected]>
@jpower432 jpower432 marked this pull request as ready for review April 12, 2024 11:40
@jpower432 jpower432 requested a review from beatrizmcouto April 12, 2024 18:12
@jpower432 jpower432 added this to the 0.9.0 milestone Apr 16, 2024
Copy link
Contributor

@beatrizmcouto beatrizmcouto left a comment

Choose a reason for hiding this comment

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

Reviewed during paired code review. Demonstrated during the meeting

@jpower432 jpower432 merged commit 6e87853 into main Apr 19, 2024
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking_change Backward incompatible changes enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add dry run option for PR validation
2 participants