Skip to content

Conversation

@zonuexe
Copy link
Member

@zonuexe zonuexe commented Aug 20, 2019

refs #129

This PR was based on #129 and made function calls asynchronous using async package. (Thanks @nanasess)

@nanasess
Copy link
Contributor

@zonuexe It worked so well! Thanks 👍

phpactor.el Outdated
(unless (file-exists-p (expand-file-name ".gitignore" phpactor-install-directory))
(f-write-text "*\n" 'utf-8 (expand-file-name ".gitignore" phpactor-install-directory)))
(cl-loop for file in '("composer.json" "composer.lock")
(cl-loop for file in '("composer.json")
Copy link
Contributor

Choose a reason for hiding this comment

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

This one leaves me doubtful. Why remove composer.lock and why do it here ?

@kermorgant
Copy link
Contributor

Hi @zonuexe @nanasess
good to see this work, thanks :-)
Not sure I can provide any insight about the code but I will try it tomorrow.

@kermorgant
Copy link
Contributor

just tested, seems to work well.

On code level, I will trust you as I'm not used to async and a bit short of time to dive into it :-)

Only thing that worries me is that line with composer.lock being removed : as far as I understand, its purpose is to get identical installations of dependencies for everyone, and that sounds to me as a good thing, so what's the reason to remove it ?

@zonuexe
Copy link
Member Author

zonuexe commented Aug 23, 2019

composer.lock is essentially unnecessary if composer.json is properly described. There is a trouble caused by this file being present. However, since it was found that simply erasing would cause another problem for existing users, we will deal with it again in another PR.

@zonuexe zonuexe merged commit f846453 into master Aug 23, 2019
@zonuexe zonuexe deleted the use-async-v2 branch August 23, 2019 12:19
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