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

refactor: converted Utils class in module #53

Conversation

roccoemanuelebonanno
Copy link
Contributor

@roccoemanuelebonanno roccoemanuelebonanno commented Apr 7, 2024

In this PR, I have converted the Utils class into a module because, in my opinion, using a class is unnecessary in this case. "Modules are mainly used to group common functionalities and enable their reuse."

@roccoemanuelebonanno roccoemanuelebonanno force-pushed the build/change-utils-class-in-module branch 2 times, most recently from dd8f4de to ed078d8 Compare April 7, 2024 12:43
@roccoemanuelebonanno roccoemanuelebonanno force-pushed the build/change-utils-class-in-module branch 2 times, most recently from 7a30dbb to c34889e Compare April 22, 2024 06:48
lib/itax_code/encoder.rb Outdated Show resolved Hide resolved
lib/itax_code/omocode.rb Outdated Show resolved Hide resolved
lib/itax_code/parser.rb Outdated Show resolved Hide resolved
lib/itax_code/version.rb Outdated Show resolved Hide resolved
lib/itax_code/utils.rb Outdated Show resolved Hide resolved
@matteoredz
Copy link
Owner

@roccoemanuelebonanno the PR title should be updated too. While it's correctly applying the conventional commits, the changes here are not about build but more chore, refactor, or perf.

@matteoredz
Copy link
Owner

Also, to make the Rubocop linter happy, we should exclude the new Utils module from the following rule:

lib/itax_code/utils.rb:7:3: C: Metrics/ModuleLength: Module has too many lines. [121/100]
  module Utils ...
  ^^^^^^^^^^^^

This can be done by moving this directive from the current location to a new rule (ab sorted):

Metrics/ModuleLength:
  Enabled: true
  Exclude:
    - 'lib/itax_code/utils.rb'

@roccoemanuelebonanno roccoemanuelebonanno changed the title build: converted Utils class in module chore: converted Utils class in module May 1, 2024
@roccoemanuelebonanno roccoemanuelebonanno changed the title chore: converted Utils class in module refactor: converted Utils class in module May 1, 2024
@roccoemanuelebonanno roccoemanuelebonanno force-pushed the build/change-utils-class-in-module branch from c34889e to 92691ac Compare May 1, 2024 10:14
@roccoemanuelebonanno roccoemanuelebonanno force-pushed the build/change-utils-class-in-module branch from 92691ac to 5dc6150 Compare May 2, 2024 08:52
Copy link
Owner

@matteoredz matteoredz left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the PR 🥳

@matteoredz
Copy link
Owner

I see an error from the CodeClimate check, but it's unrelated to this PR. I'll address it in another context.

@matteoredz matteoredz merged commit 99dddee into matteoredz:main May 2, 2024
9 of 10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants