-
Notifications
You must be signed in to change notification settings - Fork 416
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
Typos adjusts #699
Typos adjusts #699
Conversation
Thank you @djpremier. Great PR! Code, test and docs fixes! You even removed the trailing spaces at EOL. (RubyMine has a built-in option for that but it ignores PR split
RematchThe If you run all files with the rebuild option, you rebuild also the files that didn't fail, thus some value may get reordered inside the new file, despite the actual values are the same. Besides you might permanently hide failures by storing the failed value as the expected value, if you didn't get a chance to check it first. A safer way to update the |
Thanks for the feedback @ddnexus 😊. As for removing spaces, it is automatic when I save a file hehe. As I use VS Code, I adjusted 2 settings in it to avoid conflicts and remove these unwanted "things", they are: RematchYes, I only left what related to my changes, my question was whether there was anything I should do to maintain order. I will analyze your library when possible, maybe there is something we can adjust so that the result is always the same for the same content. |
As for gems, it's a habit of mine to try to keep projects as up to date as possible, to avoid having too many changes to keep track of later, but if you already have your way you can maintain it without any problems. I created the new PR for the code fixes and only uploaded the update necessary for the CI to work. |
Minitest doesn't run the tests in order, because in the author opinion tests should be atomic and shouldn't need any particular sorting in order to pass. I have to agree. That means that instead of storing/reading the values from the store file in sync to their executions, we would need to keep the values in some variable. When the test file is done running, sort the values according to the location of the rematch call and write the ordered store file. We would also have to consider what to do when you just reorder the tests. Should it fail or not? Should we rewrite the store file regardless (for consistency)? Automatically? Writing while we are supposed to read? Should we just warn about it? It looks like we would add quite a lot of complexity. All that is possible, but I am not sure the benefit of having the ordered store file justifies the overhead and the added maintenance burden, but if you find a better way to do it... you will be my hero! 🙂 |
Hi,
These are some small errors that I found while updating my application. If you need it to be in different PRs, I can do that too.
@ddnexus one question I got is how you are running the command to regenerate the Rematch files, looking in its documentation I found the command
rake test TESTOPTS=--rematch-rebuild
, however with this all the files were changed basically in the order in which tests are saved, would this be expected? Or is there some command that forces a certain ordering?Complete command I used (it can even be mentioned in the contribution doc for those who don't "know" how to run it):
docker run -it --rm --name pagy-script -v "$PWD":/usr/src/pagy -w /usr/src/pagy ruby:latest /bin/bash -c "bundle install && rake test TESTOPTS=--rematch-rebuild"
Example of result with divergence:
