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

TargetAction script fix #997

Merged
merged 8 commits into from
Feb 12, 2020
Merged

TargetAction script fix #997

merged 8 commits into from
Feb 12, 2020

Conversation

fortmarek
Copy link
Member

@fortmarek fortmarek commented Feb 11, 2020

Resolves #994

Short description 📝

As @kwridan has rightly pointed out, in order for TargetAction that invokes a script to work, you have to use ./name_of_your_script.sh, rather than name_of_your_script.sh in the build phase - but you can not do that for TargetAction with path (as it will convert it to without ./ prefix)

Solution 📦

Adding ${PROJECT_DIR} to path of the script resolves the issue - both versions should now work.

Implementation 👩‍💻👨‍💻

Detail in a checklist the steps that you took to implement the PR.

  • Add ${PROJECT_DIR} prefix
  • Add fixture test

@codecov
Copy link

codecov bot commented Feb 11, 2020

Codecov Report

Merging #997 into master will increase coverage by 0.16%.
The diff coverage is 97.38%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #997      +/-   ##
==========================================
+ Coverage    78.8%   78.96%   +0.16%     
==========================================
  Files         199      200       +1     
  Lines       10821    10920      +99     
==========================================
+ Hits         8527     8623      +96     
- Misses       2294     2297       +3
Impacted Files Coverage Δ
Sources/TuistGenerator/Linter/GraphLinter.swift 97.84% <100%> (ø) ⬆️
...stGenerator/Linter/StaticProductsGraphLinter.swift 97.24% <97.24%> (ø)

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 95e9082...e443701. Read the comment docs.

Copy link
Collaborator

@kwridan kwridan left a comment

Choose a reason for hiding this comment

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

👍

@pepicrft
Copy link
Contributor

Thanks for tackling this @fortmarek. Your changes look good to me.

@kwridan
Copy link
Collaborator

kwridan commented Feb 12, 2020

The acceptance tests are failing due the script file permissions - I believe it needs to be executable.

@fortmarek
Copy link
Member Author

The acceptance tests are failing due the script file permissions - I believe it needs to be executable.

The Project.swift fixture was wrongly setup (.pre(path: "/bin/echo"...), but it should have been .pre(tool: "/bin/echo"...))

@fortmarek fortmarek merged commit 73a6037 into master Feb 12, 2020
@natanrolnik natanrolnik deleted the bugfix/target_action branch June 17, 2020 16:33
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.

TargetAction script file
3 participants