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

Fix generating todo when todo file already exists #587

Conversation

mrbiggred
Copy link
Contributor

@mrbiggred mrbiggred commented Oct 18, 2023

Fixes #565 and should allow PR #176 to be closed. To duplicate the bug:

  1. Run standardrb --generate-todo on a project with some linting errors.
  2. Check the .standard_todo.yml file to see what linting rules are ignored. For example:
ignore:
- lib/standard.rb:
  - Layout/IndentationConsistency
  1. Run standardrb --generate-todo a second time. Without this fix it would not list the existing linting rules in the old .standard_todo.yml file. for example, the new file would NOT contain:
ignore:
- lib/standard.rb:
  - Layout/IndentationConsistency

The fix was to skip the loading of an existing .standard_todo.yml file generating the new one. Let me know what you think and if you have any feedback to improve this PR.

This commit refactors the `builds_config.rb` file to handle generating a new todo file. It adds logic to not load the existing todo file when generating a new one, ensuring that the new todo file includes all necessary ignore rules from scratch.
…e rules.

- Update `BuildsConfigTest` to remove the `test_todo_merged` method
- Removed fixture "u" files.
This commit adds a new test case to ensure that the todo file is not loaded when generating the todo file.
- Refactored the code to use `unless` instead of checking if `argv` does not include "--generate-todo"
- Refactored the test to remove unneeded expected value setup.
The commit adds a new test to check whether standardrb's '--generate-todo' generates the correct todo file.
New test case is introduced to validate the to-do list generation when one already exists. It ensures that existing errors will not obstruct the to-do list generation and will be duly reported.
- Refactored the before_setup method to include additional setup steps.
- Updated the generate_todo_existing test to use a separate test folder and fixture folder for better organization.
- Modified file paths in the generate_todo_existing test to reflect the new folder structure.
- Refactored the `test_generate_todo` and `test_generate_todo_existing` methods in the `StandardrbTest` class to use a new helper function called `assert_generate_todo`.
@mrbiggred
Copy link
Contributor Author

@searls and @knutsenm just bumping this PR. Let me know if you have any feedback and/or if there is anything you need from me to get this merged. Please also let me know if you are busy and I will ping you again at a later date of your choosing.

@searls
Copy link
Contributor

searls commented Dec 4, 2023

LGTM. Sorry for missing this prior

@searls searls merged commit 0608713 into standardrb:main Dec 4, 2023
5 checks passed
@searls
Copy link
Contributor

searls commented Dec 4, 2023

Landed in 1.32.1

@knutsenm
Copy link

knutsenm commented Dec 4, 2023

Thank you!

@mrbiggred
Copy link
Contributor Author

Wow, that was fast. Thank for you for merging the fix and ping me if there are any issues now that it's merged.

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.

standardrb --generate-todo replaces any existing todo file with deltas only
3 participants