-
Notifications
You must be signed in to change notification settings - Fork 40
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
Add auto-start support #106
Conversation
3034149
to
c088711
Compare
I took the time (sorry for that, I had problem to understand ERT) but now it should be ready to be merged if you want :) |
Hey, looks pretty OK. Some of the formatting could use some love :) I'm not sure how I feel about the explicit call to the auto start method. I think the idea was to have it auto-start immeiately after What should happen if I call the auto start function and then define another auto-start process during my session? We should at least explain the expected behaviour. |
Maybe we could use |
I have a look at this (about the formatting too :D) and I update the pull request |
Hello, I had a little bit of time (lately) to rethink about it:
|
(prodigy-start-service service | ||
(lambda () | ||
(prodigy-stop-service service nil done)))))) | ||
(with-sandbox |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ert-deftest-async
is a macro defined in some helper file. You need to load that file before reindenting as that will also load the macro's indent rule. This is most certainly not correct.
6d28856
to
0d07429
Compare
I refactor some stuff:
|
@seblemaguer Thanks, looks good! I'll try to review this ASAP but please if I forget don't be afraid of being annoying and just ask for review! That helps me a lot to keep track of everything. |
@Fuco1 can we see to finish this pr? Else I am going to loose track of it for months again :/ |
Please don't have inline comments, better describing them with a test. Comments get out of date. |
@rejeep I removed the comments |
Hello, is there any news for this? |
ed64668
to
c4a9617
Compare
c4a9617
to
96b6d5b
Compare
I'm closing all old PRs. If you still care, please rebase your branch and reopen the PR. |
With context.el this pr became obsolete. Sorry I forgot to close it |
I am currently working on adding the auto-start support.
This aims to solve #38