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/linter contacts methods #1611

Merged
merged 2 commits into from
Aug 13, 2024
Merged

Feat/linter contacts methods #1611

merged 2 commits into from
Aug 13, 2024

Conversation

leogermani
Copy link
Contributor

@leogermani leogermani commented Aug 13, 2024

All Submissions:

Changes proposed in this Pull Request:

#1594 done again after its base epic branch got merged leaving it behind

In the effort of consolidating our data flows, we want to enforce that every contact update goes through the same route, starting at Newspack_Newsletters_Contacts class to make sure we have the propper logging.

However, we can't make all the internal methods private or protected.

This PHP Sniffer helps us avoid adding any new code that will bypass our intended flow. We'll need to add this sniffers to other Newspack plugins as well. - TBD how to do that

How to test the changes in this Pull Request:

  1. checkout this branch
  2. run ./vendor/bin/phpcs
  3. Confirm you see some error in 2 files. (These errors are fixed in Add/logging #1574)
  4. Does the message look good?
  5. Try to add a method to one of the forbidden methods anywhere in the code.
  6. Make sure the sniffer catches it

PS - the linting errors in the CI job are expected until we merge #1574

Other information:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes, as applicable?
  • Have you successfully ran tests with your changes locally?

Copy link

codecov bot commented Aug 13, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 19.60%. Comparing base (dd56ab8) to head (550be52).
Report is 33 commits behind head on trunk.

Additional details and impacted files
@@            Coverage Diff            @@
##              trunk    #1611   +/-   ##
=========================================
  Coverage     19.60%   19.60%           
  Complexity     2370     2370           
=========================================
  Files            45       45           
  Lines          8922     8922           
=========================================
  Hits           1749     1749           
  Misses         7173     7173           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@leogermani leogermani merged commit 86477f6 into trunk Aug 13, 2024
10 checks passed
@leogermani leogermani deleted the feat/linter-contacts-methods branch August 13, 2024 17:07
@matticbot
Copy link
Contributor

🎉 This PR is included in version 3.1.0-alpha.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@matticbot
Copy link
Contributor

🎉 This PR is included in version 3.1.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants