Skip to content

Conversation

@weshinsley
Copy link
Contributor

Submission Checklist

  • Run unit tests
  • Declare copyright holder and agree to license (see below)

(Unable to run unit tests fully locally - but the fix in this PR is very minor and all within strings)

Summary

Using grep on windows, the filename and match string need to be provided (as I see it) with double-quotes, not single-quotes. Without that, I am seeing grep failures pulling out the metadata. See comments on #660 which is also working in that section of code, where Sys.which() is used to determine where grep.exe is.

Copyright and Licensing

Please list the copyright holder for the work you are submitting
(this will be you or your assignee, such as a university or company):
Imperial College London

By submitting this pull request, the copyright holder is agreeing to
license the submitted work under the following licenses:

@weshinsley weshinsley changed the title Wes/grepquotes Use double-quoting when calling grep.exe on Windows Jun 29, 2022
Copy link
Member

@rok-cesnovar rok-cesnovar left a comment

Choose a reason for hiding this comment

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

Looks good. Thanks!

@codecov-commenter
Copy link

Codecov Report

Merging #663 (e117e73) into master (743bf8f) will not change coverage.
The diff coverage is 0.00%.

@@           Coverage Diff           @@
##           master     #663   +/-   ##
=======================================
  Coverage   89.77%   89.77%           
=======================================
  Files          12       12           
  Lines        3529     3529           
=======================================
  Hits         3168     3168           
  Misses        361      361           
Impacted Files Coverage Δ
R/csv.R 94.83% <0.00%> (ø)

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 743bf8f...e117e73. Read the comment docs.

@rok-cesnovar rok-cesnovar merged commit 953fdd0 into stan-dev:master Jun 29, 2022
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.

3 participants