Skip to content
This repository has been archived by the owner on Jul 12, 2022. It is now read-only.

[Fix] - Searching only for run.sh on docker pre run #598

Merged
merged 2 commits into from
Oct 21, 2020
Merged

[Fix] - Searching only for run.sh on docker pre run #598

merged 2 commits into from
Oct 21, 2020

Conversation

JoaoDanielRufino
Copy link
Contributor

Signed-off-by: JoaoDanielRufino joaodaniel0405@gmail.com

- What I did
When building a formula with docker on windows, the CLI only checks if there is a run.bat file, but on docker it is necessary to check if there is a run.sh file to build the formula.

- How to verify it
Build a formula locally on windows and then run the formula with docker.

- Description for the changelog
Check if there is run.sh file when building formula with docker.

Signed-off-by: JoaoDanielRufino <joaodaniel0405@gmail.com>
@codecov-commenter
Copy link

Codecov Report

Merging #598 into master will decrease coverage by 0.50%.
The diff coverage is 50.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #598      +/-   ##
==========================================
- Coverage   80.34%   79.83%   -0.51%     
==========================================
  Files          98       98              
  Lines        3287     3219      -68     
==========================================
- Hits         2641     2570      -71     
- Misses        462      465       +3     
  Partials      184      184              
Impacted Files Coverage Δ
pkg/formula/formula.go 80.00% <0.00%> (-5.72%) ⬇️
pkg/formula/runner/docker/pre_run.go 85.71% <100.00%> (ø)
pkg/formula/watcher/watcher.go 72.97% <0.00%> (-5.41%) ⬇️
pkg/formula/runner/inputs.go 98.06% <0.00%> (-1.02%) ⬇️

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 555ee9d...d7cfbd4. Read the comment docs.

@brunasilvazup brunasilvazup added 🪲 bug Report a bug encountered while operating Ritchie ✔️ ready-for-review ready for review 💠 Windows This issue or pull request is specific to the Windows operating system labels Oct 8, 2020
Copy link
Contributor

@brunasilvazup brunasilvazup left a comment

Choose a reason for hiding this comment

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

@JoaoDanielRufino this change its necessary only in windows? If yes, would it be good to specify windows situation in the code?

PS.: I used 'request changes' to discuss this before, not necessarily needs changes.

@brunasilvazup
Copy link
Contributor

@JoaoDanielRufino this change its necessary only in windows? If yes, would it be good to specify windows situation in the code?

PS.: I used 'request changes' to discuss this before, not necessarily needs changes.

We talked and I could understand that it is just a check for windows.

@brunasilvazup
Copy link
Contributor

/merge qa

@ritchie-bot
Copy link
Contributor

ritchie-bot bot commented Oct 8, 2020

👌 Merged branch fix/docker-pre-run into qa

@brunasilvazup brunasilvazup added 🚀 QA and removed ✔️ ready-for-review ready for review labels Oct 8, 2020
@brunasilvazup brunasilvazup merged commit b7a0546 into ZupIT:master Oct 21, 2020
brunonmelo pushed a commit to brunonmelo/ritchie-cli that referenced this pull request Nov 12, 2020
Signed-off-by: JoaoDanielRufino <joaodaniel0405@gmail.com>
Signed-off-by: Bruno N. Melo <brunonobrega.melo@gmail.com>
brunonmelo pushed a commit to brunonmelo/ritchie-cli that referenced this pull request Nov 12, 2020
Signed-off-by: JoaoDanielRufino <joaodaniel0405@gmail.com>
Signed-off-by: Bruno N. Melo <brunonobrega.melo@gmail.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
🪲 bug Report a bug encountered while operating Ritchie 💠 Windows This issue or pull request is specific to the Windows operating system
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants