Skip to content

Conversation

@ldicarlo
Copy link
Contributor

@ldicarlo ldicarlo commented Nov 8, 2020

typo ?
Actually I am not sure that I understand this sentence;
Could it be:

Even shell scripts are processed as external commands (and thus respecting shebang), so they are not covered by coverage.

(if my understanding's right)

typo ?
Actually I am not sure that I understand this sentence;
Could it be:

> Even shell scripts are processed as external commands (and thus respecting shebang), so they are not covered by coverage.

(if my understanding's right)
@codecov
Copy link

codecov bot commented Nov 8, 2020

Codecov Report

Merging #117 (6900a8f) into master (160abe5) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #117   +/-   ##
=======================================
  Coverage   66.18%   66.18%           
=======================================
  Files          98       98           
  Lines        4267     4267           
=======================================
  Hits         2824     2824           
  Misses       1443     1443           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 160abe5...6900a8f. Read the comment docs.

This is primarily designed for external command calls.
The external command does not have to be a shell script.
Even shell scripts are processed as external commands (respect shebang), so it not covered by coverage.
Even shell scripts are processed as external commands (respect shebang), so it is not covered by coverage.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Even shell scripts are processed as external commands (respect shebang), so it is not covered by coverage.
Even shell scripts are executed as external commands according to the shebang, so they are not covered by coverage.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(I added it if you want, or just close this PR, as you wish ;))

following @ko1nksm suggestion
@ko1nksm ko1nksm merged commit cca82e6 into shellspec:master Nov 10, 2020
@ko1nksm
Copy link
Member

ko1nksm commented Nov 10, 2020

Merged, thanks!

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