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

Allow users to specify executable .slate's which emit config #145

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

richo
Copy link
Contributor

@richo richo commented Nov 5, 2012

This means that you can dynamically generate your config at launch time.

My objective C fu is not great, so I'm very open to feedback on the code/don't mind refactoring.

Thanks for slate, it's awesome.

@richo
Copy link
Contributor Author

richo commented Nov 12, 2012

Bump, any news here?

@trishume
Copy link
Contributor

I believe the objective C way to do static methods is to use + instead of - in the method name. I would refactor your code to use an objective C method. Something like + (NSString*) runShellScript: (NSString*) name

@jigish
Copy link
Owner

jigish commented Nov 17, 2012

very interesting idea. i like it. +1 to @trishume though. But instead of changing that method what would be even better is if you could use ShellUtils from 9c0906b to call the script. If you can't use it, then adding a method to ShellUtils that serves your purpose and using it would be better. thanks!

@ghost ghost assigned jigish Nov 17, 2012
@richo
Copy link
Contributor Author

richo commented Jan 8, 2013

Sorry I managed to miss these comments.

It should be doable, I think ShellUtils will need a pretty drastic refactor. I'll probably need some eyes over the code, purely because my obj-c fu isn't great.

Thanks for the feedback, I'll have a loko this week.

@richo
Copy link
Contributor Author

richo commented Feb 4, 2013

Hey guys, sorry for the long delay. I finally got time to look at this and would appreciate feedback.

@richo
Copy link
Contributor Author

richo commented Feb 4, 2013

Relatedly, there's potentially a deadlock in ShellUtils +run.

I'll get a patch out to you soon, it would be fairly hard to run into though.

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.

3 participants