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

Adding language option and updating swift App Delegate #201

Closed
wants to merge 4 commits into from

Conversation

TheSooth
Copy link

@TheSooth TheSooth commented Dec 4, 2014

No description provided.

@keith
Copy link
Contributor

keith commented Dec 4, 2014

👎 for me on this. I'd like to avoid this comment at the top of the file baggage.

import UIKit

@UIApplicationMain

Copy link
Contributor

Choose a reason for hiding this comment

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

I actually don't think this newline should be here since @UIApplicationMain is a decorator on the class below it.

Copy link
Author

Choose a reason for hiding this comment

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

👍 yep, this is my mistake 😢

@TheSooth
Copy link
Author

TheSooth commented Dec 4, 2014

@Keithbsmiley why u want to avoid comments on this file?

I mean - in whole app u will be have author information, except this file, so why to avoid the author info only on this file?)

@keith
Copy link
Contributor

keith commented Dec 4, 2014

I've also been removing those comments from the other files.

@TheSooth
Copy link
Author

TheSooth commented Dec 4, 2014

by default with xcode author information will be generated with new file creation, so maybe keep it as by default?

I think better to keep all files in one style

@keith
Copy link
Contributor

keith commented Dec 4, 2014

Fair enough. Overall this is @gfontenot's decision. You can always add this as a local template if you want to override the default.

@gfontenot
Copy link
Member

I'd rather not update the default template with the header comments. I don't use them in any other files, and it's fairly trivial to locally override the default template with a version that includes header comments if that's what you want.

@TheSooth
Copy link
Author

TheSooth commented Dec 4, 2014

@gfontenot k, will be deleted

@@ -10,6 +10,7 @@ def fetch_options
fetch_option_for(:company_identifier, 'Company identifier')
fetch_option_for(:prefix, 'Prefix')
fetch_option_for(:test_target_name, 'Test target name')
fetch_option_for(:project_template, 'Project language')
Copy link
Member

Choose a reason for hiding this comment

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

This can already be passed on the command line. I don't think we also need it in the prompt.

Copy link
Author

Choose a reason for hiding this comment

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

I think this is not user-friendly to pass language param only in command-line, 'cause user can forgot to do this and project will be generated with default language

@gfontenot
Copy link
Member

OK, been contemplating this overnight. Here's what I'm thinking:

  • AppDelegate header comments: You made a good point regarding Xcode's default templates. This is a case where I'm the one who should be using an overridden template for the app delegate. Why don't you revert that last commit and bring these back.
  • Specifying the project template in the prompt: I feel like I should clarify how I envision this being used. The default template name can be specified inside the user's liftoffrc at any level. I'd be willing to bet a significant amount of money that this doesn't need to change 95% of the time. For those times that it does, it should be an outlier, and so I think specifying the template name on the command line is exactly what you'd want. It should be a conscious decision that you're changing your default template, and it's almost certainly not something you do often enough to warrant having it present every time you create a project. So, I'd really rather not add it to the prompt.

This does bring up an interesting idea for me of allowing users to customize their prompts. I have no idea how this would work, or if it's even feasible, but it's a thought.

@gfontenot
Copy link
Member

ping @thoughtbot/ios for thoughts on the prompt issue.

@bitcrumb
Copy link
Contributor

bitcrumb commented Dec 5, 2014

I'm with @gfontenot on both points.

@keith
Copy link
Contributor

keith commented Dec 5, 2014

👍

@gfontenot
Copy link
Member

Closing in favor of #211

@gfontenot gfontenot closed this Feb 4, 2015
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.

4 participants