-
Notifications
You must be signed in to change notification settings - Fork 5.1k
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
Extract some code in packages #1449
Conversation
I think you'll need to adjust the Makefile targets and Docs in |
Hmm, given that the build ran for quite a while (even though erroneously), my comment might be false. Maybe you can double check. |
a45abd6
to
d2cfaf0
Compare
@timoreimann yep, I need to update docs, just want to make sure these changes are ok for @emilevauge and everybody before doing it 👼 |
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.
LGTM beside docs 👼 Good Job!
@vdemeester Looks very good to me ( love the new |
b5a7444
to
526fc45
Compare
Updated with the docs 👼 |
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.
LGTM.
- This will help split stuff in smaller, better tested packages - This moves some stuff like the traefik command to package `cmd` Signed-off-by: Vincent Demeester <[email protected]>
526fc45
to
7fcb7b8
Compare
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.
Well done @vdemeester
LGTM 🎉
Reviving #1006 👼
cmd
It's build on top of #1444, so only the 2nd commit should be reviewed 👼
🦁