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

Autoinstall through https://github.com/phpstan/extension-installer #17

Merged
merged 6 commits into from
Sep 13, 2019
Merged

Conversation

spawnia
Copy link
Contributor

@spawnia spawnia commented Sep 11, 2019

No description provided.

@szepeviktor
Copy link

What a modernization!

@szepeviktor
Copy link

szepeviktor commented Sep 11, 2019

Try using composer global require localheinz/composer-normalize

szepeviktor
szepeviktor previously approved these changes Sep 11, 2019
@szepeviktor
Copy link

szepeviktor commented Sep 11, 2019

  • localheinz/composer-normalize is not a dev dependency, it needs to be installed globally
  • GitHub's syntax highlight does not understand "neon", only "yaml"

@mazsudo
Copy link
Contributor

mazsudo commented Sep 11, 2019

thx @spawnia for contributing. agreed with @szepeviktor on the 2 points. I would add don't forget to update Changelog and if you can squash your commit that would be great. thx again for this modernization 😄

@spawnia
Copy link
Contributor Author

spawnia commented Sep 11, 2019

I would argue it is a dev dependency, as dev's are expected to run it and it is useful to validate in CI. There is no hard requirement that forces it to be installed globally.

@spawnia
Copy link
Contributor Author

spawnia commented Sep 11, 2019

You can squash on merge, the title of the PR should make a good commit message.

@spawnia
Copy link
Contributor Author

spawnia commented Sep 12, 2019

@mremi mremi merged commit 5d7853d into ekino:master Sep 13, 2019
@mremi
Copy link
Member

mremi commented Sep 13, 2019

Thanks @spawnia for the contribution!

About commits, of course we can squash them on merge, but it's easier for the merger if the submitted PR is "clean", because no need talking about that, and the author gets the opportunity to update the commit message to have a proper one.

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