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

travis: fail if any cask files added outside Casks dir #26819

Merged
merged 1 commit into from
Nov 16, 2016
Merged

travis: fail if any cask files added outside Casks dir #26819

merged 1 commit into from
Nov 16, 2016

Conversation

jawshooah
Copy link
Contributor

@reitermarkus reitermarkus merged commit 8115771 into Homebrew:master Nov 16, 2016
@BenjaminHCCarr
Copy link
Contributor

Thank you!

As mentioned in #267915#issuecomment-260818620

I broke my own patch tree, and it looks like the patches were coming off / instead of /Casks.

Sincerely sorry for breaking Cask for everyone.

@jawshooah jawshooah deleted the check-casks-outside-casks-dir branch November 16, 2016 19:49
@jawshooah
Copy link
Contributor Author

Accidents happen 😄

@miccal
Copy link
Member

miccal commented Nov 17, 2016

Hello @jawshooah, it seems the the builds are failing for all new casks. I think your lines 24 and 26 in your modified file ci/travis/script.sh shouldn't have the arithmetic tests arg -gt 0 but instead should have -n arg?

Edit: It is passing for modified Casks, so the problem is not what I thought.

@BenjaminHCCarr
Copy link
Contributor

I'm seeing this in #26845 as well if you want an example......
cc: @jawshooah @reitermarkus @vitorgalvao @MikeMcQuaid

@jawshooah
Copy link
Contributor Author

@miccal The arithmetic test isn't the problem; it's just checking if the array casks_outside_casks_dir is non-empty. Testing the length with -gt, -lt, etc is less error-prone than string tests like -n, -z, etc because they only operate on the first element of the array. See the answers here.

The problem is that git diff *.rb is pulling in files outside the current working directory. Using ./*.rb instead doesn't seem to change anything. Will investigate further later, but in the meantime this can be reverted.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants