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

Support umbrella apps #1

Merged
merged 3 commits into from
Apr 26, 2019
Merged

Support umbrella apps #1

merged 3 commits into from
Apr 26, 2019

Conversation

liskin
Copy link
Contributor

@liskin liskin commented Apr 26, 2019

Fix root path detection for umbrella apps, plus two small cleanups.

mix.exs Outdated Show resolved Hide resolved
@remi
Copy link
Member

remi commented Apr 26, 2019

This is awesome! 🎉

If you have time to add both Elixir versions (1.7 and 1.8) to .travis.yml so they are both officially tested, I’ll merge it right after.

If you don’t have time, no worries, I’ll merge it right now and edit .travis.yml afterwards.

Thank you!

@liskin liskin force-pushed the umbrella branch 2 times, most recently from 53ebd91 to 26c6deb Compare April 26, 2019 21:00
It works with older Elixir just fine. :-)
Just a harmless cleanup.
@remi
Copy link
Member

remi commented Apr 26, 2019

This is great, thank you Tomáš! I’ll cut the 0.2.0 right after this.

@remi remi merged commit 0b6af08 into mirego:master Apr 26, 2019
@liskin liskin deleted the umbrella branch April 29, 2019 08:06
@liskin
Copy link
Contributor Author

liskin commented Apr 29, 2019

Thanks for merging.

BTW, I'm new to Elixir and its community and I've been wondering if merging via squashing is the standard here because this is my second Elixir-related PR that was merged in that way and it's making me somewhat uncomfortable as there's now a squashed commit authored by me in some git history somewhere and I always strived to make proper commits that don't mix unrelated changes... But if this is standard in the Elixir community, I can live with that, I guess. :-)

@remi
Copy link
Member

remi commented Apr 30, 2019

BTW, I'm new to Elixir and its community and I've been wondering if merging via squashing is the standard here because this is my second Elixir-related PR that was merged in that way

I think it’s just a coincidence :)

It's making me somewhat uncomfortable as there's now a squashed commit authored by me in some git history somewhere and I always strived to make proper commits that don't mix unrelated changes...

This is just my personal preference as an open-source project maintainer. I always try to keep a clean git history in the master branch. For instance when you look at the history for this repo, you see a Support umbrella apps commit, not all the individual commits that amounted to the whole feature (eg. refactors from PR comments, minor stuff like formatting, etc.).

Thank you for contributing!

@liskin
Copy link
Contributor Author

liskin commented Apr 30, 2019

Okay, thanks for clarifying. :-)

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.

None yet

2 participants