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

Allows user not to clean on ipa command #168

Merged
merged 1 commit into from Apr 2, 2015

Conversation

ashfurrow
Copy link
Contributor

Shenzhen cleans by default unless you specify the --no-clean flag. This is a problem, since some projects (ours) cannot be successfully compiled on a clean build.

The way the ipa command was set up lead to basically no being able to opt out of the default clean.

There's probably a lot better way to do this, both syntactically and from a "this is terrible code" perspective. I couldn't figure out how to use clean: false or something equivalent, but would be happy to do so given a point in the right direction.

Also I can add tests of course, when we get the implementation down.

@KrauseFx
Copy link
Member

KrauseFx commented Apr 2, 2015

Makes sense, it's okay to add it like this. If you could add a test for it, it's ready to merge 👍

@ashfurrow
Copy link
Contributor Author

Sounds good – I'll add a test.

It does feel a bit unnatural though. Would it be better to have a clean value that is true by default?

@KrauseFx
Copy link
Member

KrauseFx commented Apr 2, 2015

Yes, probably! Let's go with that.

@ashfurrow
Copy link
Contributor Author

OK cool. I'll come up with an implementation I think make sense and some tests.

@KrauseFx
Copy link
Member

KrauseFx commented Apr 2, 2015

Thanks @ashfurrow for working on this 👍

@ashfurrow
Copy link
Contributor Author

No problem! Before I write any tests, does this implementation make sense? Any suggestions on its style? Couldn't figure out how to switch between two different args depending on the value of the param.

@KrauseFx
Copy link
Member

KrauseFx commented Apr 2, 2015

Yes, it looks good. I honestly don't like the style of this part of the code in general (not yours, the code as it was before), as it's not very clear what part is being used.

If it works, I'm okay with it 👍

@ashfurrow
Copy link
Contributor Author

OK cool. I'll write a test, then!

@ashfurrow
Copy link
Contributor Author

OK coo, this is tested.

Users providing the old clean: nil argument will need to change. I can add a check for v.nil? for backwards compatibility. Just let me know.

@joshdholtz
Copy link
Member

Looks good to me 👍

KrauseFx added a commit that referenced this pull request Apr 2, 2015
Allows user not to clean on ipa command
@KrauseFx KrauseFx merged commit 641daaf into fastlane:master Apr 2, 2015
@KrauseFx
Copy link
Member

KrauseFx commented Apr 2, 2015

Thanks again @ashfurrow 🚀

@KrauseFx
Copy link
Member

KrauseFx commented Apr 2, 2015

This will be in the next release, please use the latest version from the master branch for now.

@ashfurrow
Copy link
Contributor Author

Coooool will do!

KrauseFx added a commit that referenced this pull request Mar 7, 2016
Issue #168 wrap xcodebuild calls within ruby system environment
@fastlane fastlane locked and limited conversation to collaborators Feb 4, 2017
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.

None yet

3 participants