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.

Sign up for GitHub

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

Jump to bottom

Restore solidus/tax_category_deprecated warning cop #63

Merged
merged 3 commits into from
Nov 24, 2023
Merged

Restore solidus/tax_category_deprecated warning cop #63

merged 3 commits into from
Nov 24, 2023

Conversation

safafa
Copy link
Collaborator

@safafa safafa commented Nov 7, 2023 •
edited
Loading

Description

This PR restores solidus/tax_category cop
issue 53

Severity:

  • - info
  • - refactor
  • - convention (default)
  • - warning
  • - error
  • - fatal

Wrong Code

  model.tax_category = data

Correct Code

  model.tax_categories = [data]

Solidus PR Link: # Replace with link to Solidus PR where the change has been made.

Before submitting the PR make sure the following are checked:

  • The PR relates to only one cop with a clear title and description.
  • Feature branch is up-to-date with main (if not - rebase it).
  • Squashed related commits together.
  • Added tests.
  • Ran and ensured all tests are passing on a development environment.
  • If this is a new cop, added an entry for the cop on /config/default.yml
  • Updated Changelog

@safafa safafa force-pushed the safa/restore-tax-category-deprecated-cop branch 2 times, most recently from a5f94fa to 103a8ce Compare November 7, 2023 12:14
@safafa safafa marked this pull request as ready for review November 7, 2023 12:34
MassimilianoLattanzio
MassimilianoLattanzio approved these changes Nov 8, 2023
View reviewed changes
Copy link
Collaborator

@MassimilianoLattanzio MassimilianoLattanzio left a comment

Choose a reason for hiding this comment

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

Great job! Everything looks good.

I just left some comments regarding some nitpicks.

changelog/new_add_solidus_tax_category_deprecated.md Outdated Show resolved Hide resolved
config/default.yml Show resolved Hide resolved
spec/rubocop/cop/solidus/tax_category_deprecated_spec.rb Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
relnotes/v0.3.0.md Outdated Show resolved Hide resolved
@safafa safafa force-pushed the safa/restore-tax-category-deprecated-cop branch 2 times, most recently from cd5a56e to 4f9c92a Compare November 10, 2023 10:40
@safafa
Copy link
Collaborator Author

safafa commented Nov 10, 2023

Thank you @MassimilianoLattanzio I made all the requested changes here

@safafa safafa self-assigned this Nov 10, 2023
@safafa safafa force-pushed the safa/restore-tax-category-deprecated-cop branch from 4f9c92a to 747aea3 Compare November 24, 2023 16:34
@safafa safafa merged commit 91dec22 into main Nov 24, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DanielePalombo DanielePalombo DanielePalombo approved these changes

@MassimilianoLattanzio MassimilianoLattanzio MassimilianoLattanzio approved these changes

Assignees

@safafa safafa

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@safafa @DanielePalombo @MassimilianoLattanzio