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

Fix issues with lizard executable logic #8

Merged
merged 1 commit into from
Mar 19, 2018

Conversation

mgrebenets
Copy link
Collaborator

  • Lizard executable expects the following:
    • lizard [options] [PATH or FILE] [PATH] ...
    • But current code puts PATH (source_directory) before the options
  • The check for 'which lizard' ignores 'params[:executable]' option

@mgrebenets
Copy link
Collaborator Author

@liaogz82 can't figure out how to add reviewers, so pinging you instead 👍

@mosesliao mosesliao self-requested a review March 19, 2018 02:12
@mosesliao
Copy link
Owner

Give me some time to check the CI review. thanks

@mosesliao
Copy link
Owner

wait... you are PRing from your own fork. The CircleCI is not running because of that.

let me google for solutions to the CI problem. May take a bit longer. Please don't remove the fork for now

@mgrebenets
Copy link
Collaborator Author

Hm... interesting, I've been using forks before on other open-source repos.
It should have picked it up, the changes are on my fork, but the PR is on main repo.

@mosesliao
Copy link
Owner

mosesliao commented Mar 19, 2018

screen shot 2018-03-19 at 10 20 30 am

Could you try add a comment and do a git push -f or something? thanks.

it is a circleCI setting. it was off just now

@mosesliao
Copy link
Owner

ok running

@mgrebenets
Copy link
Collaborator Author

Good point on CI part, I'll check if I can add any specs for this change.

@mosesliao
Copy link
Owner

oh btw, the rake release will be done manually once the bug get solved.

@mosesliao
Copy link
Owner

https://circleci.com/gh/liaogz82/fastlane-plugin-lizard/25

strange. it is passing here.

@mgrebenets
Copy link
Collaborator Author

Strange indeed, because I definitely broke the test that checks the order of input folder and -l swift option 🤔

I've just pushed updates with new rspecs and fixed rspec for position of source folder.

@mgrebenets
Copy link
Collaborator Author

Wow, that's fast, but this CI run was using new tests:

    when specify custom executable
      uses custom executable
      should raise if custom executable does not exist <-- this one's new

- Lizard executable expects the following:
  - lizard [options] [PATH or FILE] [PATH] ...
  - But current code puts PATH (source_directory) _before_ the options
- The check for 'which lizard' ignores 'params[:executable]' option
@mgrebenets mgrebenets self-assigned this Mar 19, 2018
Copy link
Owner

@mosesliao mosesliao left a comment

Choose a reason for hiding this comment

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

good job @mgrebenets and thanks for fixing up the issue.

Tonight at singapore time I will roll out v1.0.2. Let me know if you have an issue with the versioning

@mgrebenets mgrebenets merged commit 25722b2 into mosesliao:master Mar 19, 2018
@mgrebenets
Copy link
Collaborator Author

@liaogz82 Sounds good to me.
Thanks 🎉

@mgrebenets mgrebenets deleted the executable-issues branch March 19, 2018 06:06
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