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

CPLAT-6431 Add option to skip overlapping patches #22

Merged
merged 6 commits into from
Jul 10, 2019

Conversation

smaifullerton-wk
Copy link
Contributor

@smaifullerton-wk smaifullerton-wk commented Jul 8, 2019

Motivation

I'm writing a codemod where it is inevitable that some patches will overlap. I'd like to be able to optionally skip overlapping patches instead of throw an exception when it happens. Also, I'd like to be able to output what was skipped at the end of the code modification for consumers to be able to manually edit should they so desire.

Changes

  • Before applying patches, check each one to see if there are any overlaps. If there are, prompt the user to either 1.) skip the patch, or 2.) quit the codemod.
  • The skipped patches will be removed from the list of applied patches passed to applyPatchesAndSave. When overlapping patches are passed to applyPatches, the current behavior of throwing an exception will stay the same. It's just that now when running a codemod, we'll always pull them out (or quit early) before giving them to the apply methods.

Release Notes

Review

See CONTRIBUTING.md for more details on review types (+1 / QA +1 / +10) and code review process.

Please review:

QA Checklist

  • CI passes

Merge Checklist

While we perform many automated checks before auto-merging, some manual checks are needed:

  • A Client Platform member has reviewed these changes
  • There are no unaddressed comments - this check can be automated if reviewers use the "Request Changes" feature
  • For release PRs - Version metadata in Rosie comment is correct

@aviary3-wk
Copy link

Security Insights

No security relevant content was detected by automated scans.

Action Items

  • Review PR for security impact; comment "security review required" if needed or unsure
  • Verify aviary.yaml coverage of security relevant code

Questions or Comments? Reach out on Slack: #support-infosec.

lib/src/util.dart Outdated Show resolved Hide resolved
@smaifullerton-wk smaifullerton-wk changed the title Add option to skip overlapping patches CPLAT-6431 Add option to skip overlapping patches Jul 10, 2019
@todbachman-wf
Copy link
Member

lgtm

skippedPatches.addAll(userSkipped);

// Don't apply the patches the user skipped.
for (var patch in userSkipped) {
Copy link
Contributor

Choose a reason for hiding this comment

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

#nit could put this logic in a local function since it's duplicated here and below

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Gonna leave the nit -- I'd prefer not to modify the instance of appliedPatches and skippedPatches within a helper function as there’s already a lot of scope to keep in mind in _runInteractiveCodemod.

@evanweible-wf
Copy link
Contributor

QA +1

  • CI passes and verified the overlap prompting locally

@Workiva/release-management-p

@rmconsole4-wk rmconsole4-wk merged commit 846b45d into master Jul 10, 2019
@rmconsole4-wk rmconsole4-wk deleted the add-option-to-skip-overlaps branch July 10, 2019 18:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants