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

Code cleanup and linter fixes #46

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from
Draft

Conversation

bmos
Copy link
Contributor

@bmos bmos commented Feb 28, 2024

I ran the code through rubocop and ignoring just the line length warnings. You can keep this or leave it, I know code style can be very subjective.

It depends on my other commit #45 and would be easiest to merge if you first merge #45 via rebase and merge.

EDIT: Also the frozen string literal feature at the top of the files doesn't work for at least one file.

Was throwing `TypeError: no implicit conversion of String into Integer`
was throwing ``DEPRECATION WARNING: to_h is deprecated and will be removed from Rails 7.0 (You can use `ActiveRecord::Base.configurations.configs_for(env_name: 'env', name: 'primary').configuration_hash` to get the configuration hashes.) (called from load at /home/.../ruby/3.2.0/bin/pluto:25)``
@geraldb
Copy link
Member

geraldb commented Feb 29, 2024

@bmos thanks for taking the time and the effort. sorry about rubocop i am with the japenese rubyists to quote:

We don't use RuboCop because we can manage our coding style by ourselves. We want to accept small fluctuations in our coding style because we use Ruby.
Please do not submit issues and PRs that aim to introduce RuboCop in this repository.

my fault. i will add a note to the readme. happy to see a lint list in text (with warnings) to commit. i am sure there are some good parts in there and again thanks for taking the time. sorry for now my time is unfortunately limited to pick out the good ones. more than happy to work on pluto but again real life realities for now are different.

@bmos
Copy link
Contributor Author

bmos commented Feb 29, 2024

That's ok, I made sure to commit anything important to the first PR 🙂

If you're considering README changes, it would be great to also have a CONTRIBUTING.md file with instructions to get the build environment set up and any norms you want people to follow (like not linting existing code). It took me a while to figure some of that stuff out.

@geraldb
Copy link
Member

geraldb commented Feb 29, 2024

Greetings from Austria (near Vienna). Thanks for the commentary. The main idea to make contributing easier was to split the "monolith" into gems ("modul-mania") - the idea being only work / contribute to a single gem. Guess a big fail. might be more confusing. and lately i am using the monotree/repo approach / style, thus, lots of additional (experimental / optional) gems here leading to more confusion.

Anyways, again great to hear that pluto is still getting used & found (on the internets). again i try to set aside a couple of days in march 2024 (this upcoming month) to update & housekeeping. thanks for your patience.

@bmos bmos marked this pull request as draft April 7, 2024 22:27
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