'cache prepare' command #67
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅ |
|
I have read the CLA Document and I hereby sign the CLA |
|
recheck |
mscottford
requested changes
May 17, 2022
There was a problem hiding this comment.
Overall this looks really good, especially when you consider that you're not used to working with C#. You've picked up a ton of stuff to pull this PR together. 🎉
I do have some suggestions here and there. I could have done many of them myself, but I wanted to take the opportunity to share some C# knowledge where it makes sense.
CodeMouse92
force-pushed
the
cache_prepare
branch
3 times, most recently
from
2448a5b
to
4ba9193
Compare
Configures the project for BDD-based acceptance testing with Aruba and Cucumber.
...because, Freshli should be fresh, no? ;) Also updates GitHub Action tooling.
Eliminates some now-unnecessary inheritance with commands, wherein all commands would include the --output and --format options. Also updates more dependencies to the latest versions.
Implements the 'cache' command, which has a 'prepare' subcommand. At present, this does nothing.
Adds a --cache-dir global option with a default value of $HOME/.freshli, for storing temporary files.
The ScanCommandRunner.Run() function returns an integer return code (0 if successful), and the invocation of ScanCommand.Run() expects a return code, but ScanCommand.Run() incorrectly is 'void'. This changes the return of Run() so it no longer drops this return code.
Creates the database file at the user-specified (or default) location for the cache, and/or migrates the existing file. If the file is corrupt, the command fails with a non-zero exit code.
Tests for other commands will need to use the logic in VerifyAlias. This moves VerifyAlias to a new TestHelpers class, and makes the function generic, so it can be used with any System.CommandLine.Command class.
Tests for --cache-dir global option. This can only be tested on MainCommand directly, since the normal command building does not take place in the tests.
The main command should not be runnable directly. Instead, one of its subcommands must be run instead. This test confirms that as true.
Confirms the following: * 'cache' is not directly runnable (no handler) * 'cache prepare' is runnable (has handler)
Cucumber now tests more specific statements within the --help output, rather than expecting a large exact block.
Ensures that 'cache prepare' creates the expected default folder, and generates an SQLite database file that passes SQLite's "PRAGMA integrity_check". NOTE: It is not feasible to test for pending migrations with cucumber.
--format and --output are no longer shared between all commands, so all mentions of that are removed. Shows the correct inheritance for Commands now that BaseCommand<> has been removed. Adds mention of --cache-dir global argument. Documents the new cache command.
Fixes misspelled words in README.md and CONTRIBUTING.md. Improves alignment of columns on Markdown representation of tables (thanks to JetBrains). Improves readability of command list in Program.cs, to make it easier for future developers to add their own. Reordered methods in Program.cs from outermost to innermost to make navigating the file a bit easier.
Adds in Ruby 3.1 using the same setup that's used to build the official Docker container. https://github.com/docker-library/ruby/blob/master/3.1/bullseye/Dockerfile
Also changes the name of the methods to make it more clear that a value is being retrieved, not an option/argument.
Switches to using `MethodData` and `TheoryData` to represent the set of data to test. This approach is used instead of the `[InlineData]` attribute because the `ArgumentArity` values are not technically constants, and only constant values can be parameters to attributes.
|
I'm confused about why this pull request is now reporting conficts. |
Environment variables on Linux do not persist between process executions, so this was not actually enabling the 'dotnet ef' tool to work with non-standard locations. Removing all use of the environment variable to simplify code.
per PR request
|
Code Climate has analyzed commit 07de1d8 and detected 0 issues on this pull request. The test coverage on the diff in this pull request is 24.7% (50% is the threshold). This pull request will bring the total coverage in the repository to 40.1%. View more on Code Climate. |
This was erroneously left over after the collect and submit steps were separated.
mscottford
approved these changes
May 18, 2022
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This implements the following features:
--cache-dirglobal optioncachecommand (cannot be directly executed)cache preparesubcommand, which idempotently creates or verifies the existence of a fully migrated SQLite3 database in the--cache-dir.Closes #49.