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

Remove Refactoring notes from feature/cleanup and address PR comments #566

Merged
merged 6 commits into from Jun 15, 2020

Conversation

@IAmThePan
Copy link
Contributor

@IAmThePan IAmThePan commented Jun 6, 2020

  • Have you followed the guidelines in CONTRIBUTING.md?
  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?
  • Have you added an explanation of what your changes do?
  • Does your submission pass tests?
  • Did you lint your code prior to submission?

I removed all the notes I made when refactoring UNSAFE_ lifecycle events.
@christophertino Did you want to remove linting rules that are now set to 0 from .eslintrc.js?

@IAmThePan IAmThePan requested a review from ghostery/ghostery as a code owner Jun 6, 2020
Copy link
Member

@wlycdgr wlycdgr left a comment

LGTM so far. This obviates my comments re: these in the original linting PR. Will review the other changes.

@IAmThePan IAmThePan requested a review from christophertino as a code owner Jun 11, 2020
@IAmThePan IAmThePan changed the title Remove Refactoring notes from feature/cleanup Remove Refactoring notes from feature/cleanup and address PR comments Jun 11, 2020
@IAmThePan
Copy link
Contributor Author

@IAmThePan IAmThePan commented Jun 11, 2020

I tried restoring IAmThePan:feature/cleanup to put these changes where they could be tracked by the original PR but it didn't track so I created this PR first to remove the Refactoring Notes then to address all the comments from the original PR.

Copy link
Member

@wlycdgr wlycdgr left a comment

Looks good so far. Will review again once the last batch of comments from the original PR has been addressed (files from app/panel/reducers/panel.js onward to the end)

@IAmThePan IAmThePan requested a review from zarembsky as a code owner Jun 12, 2020
@IAmThePan
Copy link
Contributor Author

@IAmThePan IAmThePan commented Jun 12, 2020

I addressed all the outstanding comments on the Code Cleanup PR. Please review.

Copy link
Member

@wlycdgr wlycdgr left a comment

LGTM! Thank you for fixing the banner bug along the way!

@christophertino christophertino merged commit 51d737e into ghostery:develop Jun 15, 2020
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@christophertino christophertino added this to the 8.5.2 milestone Jun 15, 2020
@IAmThePan IAmThePan deleted the IAmThePan:feature/cleanup-cleanup branch Jun 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants