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

Allow some kw deploy commands to be run outside kernel tree #768

Closed
wants to merge 4 commits into from

Conversation

davidbtadokoro
Copy link
Collaborator

As pointed in #734, some kw deploy commands like --list and --uninstall can't be run outside a kernel tree when they should. This PR implements this.

Also, this PR contains commits fixing issues related with ShellCheck failures that I came across when implementing the above. The issues where:

  • An unreachable command in deploy.sh (SC2317))
  • Multiple individual redirects in deploy.sh (SC2129)
  • An unreachable command incorrectly pointed out by ShellCheck in deploy_test.sh (SC2129)

@codecov-commenter
Copy link

codecov-commenter commented Feb 3, 2023

Codecov Report

Base: 73.21% // Head: 73.21% // Decreases project coverage by -0.01% ⚠️

Coverage data is based on head (bb6ac3c) compared to base (73cbe52).
Patch coverage: 69.23% of modified lines in pull request are covered.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files
@@             Coverage Diff              @@
##           unstable     #768      +/-   ##
============================================
- Coverage     73.21%   73.21%   -0.01%     
============================================
  Files            41       41              
  Lines          6865     6872       +7     
============================================
+ Hits           5026     5031       +5     
- Misses         1839     1841       +2     
Impacted Files Coverage Δ
src/deploy.sh 51.35% <69.23%> (+0.20%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Previously, there were individual printf redirects to the same file,
which conflicted with rule SC2129 of ShellCheck that says to bundle
those redirects in only one (https://www.shellcheck.net/wiki/SC2129).
The reasoning is that the file is opened and closed only once and thus
there is a performance gain, although, in this case, the gain is
irrelevant being more an issue of keeping the project coding style.

Signed-off-by: David Tadokoro <davidbtadokoro@usp.br>
Before this commit, there was an unreachable command that made the
ShellCheck fail (https://www.shellcheck.net/wiki/SC2317). The fix was to
simply move the command one line before the return command (which caused
the unreachable command).

Signed-off-by: David Tadokoro <davidbtadokoro@usp.br>
As per ShellCheck documentation (https://www.shellcheck.net/wiki/SC2317)
some commands may be incorrectly considered unreachable. In this case,
the fix was made disabling this rule for the accused command.

Signed-off-by: David Tadokoro <davidbtadokoro@usp.br>
src/deploy.sh Outdated Show resolved Hide resolved
Some kw deploy commands, such as --list and --uninstall, should be able
to be executed outside a kernel tree.

The implementation revolves around these commands enabling a flag that
does not trigger the checking of an existing kernel tree in the current
directory (like the --from-package deploy command).

Important to say that the test cases associated with this implementation
only check if the flag has been set, because the checking of an existing
kernel tree is done in the deploy_main() which does not make sense to
test.

Closes kworkflow#734

Signed-off-by: David Tadokoro <davidbtadokoro@usp.br>
@rodrigosiqueira
Copy link
Collaborator

Very nice change! Tested, and merged in the unstable branch.

Thanks a lot!

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.

None yet

3 participants