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

Some improvements for Dockerfile #178

Closed
wants to merge 7 commits into from
Closed

Conversation

Savemech
Copy link

  • Bump to Ubuntu Bionic
  • Make use of DEBIAN_FRONTEND=noninteractive
  • Install, and init locale not inside python script from travis
  • Try to use poor's man solution to run this without log files, didn't work, see Add option to stream all logs to stdout #176
  • Make look more readable
  • Don't move tar-gz into layer, use unpacked from build stage

Bump to bionic
Since we're using build from master for everycommit, feel free to change branch you're interested with
Make use of `DEBIAN_FRONTEND=noninteractive`
Try to use poor's man solution to run this withou log files, didn't work, see zulip#176
@timabbott
Copy link
Member

@Savemech thanks for the contributions! I think some of the changes (e.g. removing comments) are probably counterproductive for users who are not experts in using Docker. Some of the other changes (e.g. readability improvements) seem nice and I'd be happy to integrate if they were in separate commits. Would you be up for reorganizing this branch to start with the most easily mergable changes in separate commits (e.g. readability improvements) and do one commit per distinct fix?

Check out our GitHub guide and commit guidelines for more details.

- Added comments back
- Added more comments, also highlight some problems and ways to improve
@Savemech
Copy link
Author

Add some comments, and maybe I would check some problems that highlighted, and find some way to improve size of resulting image(that bothers me a lot), because were building Zulip for every commit in master automatically and for now its average of ~45 mins

@Savemech
Copy link
Author

Savemech commented Jan 3, 2019

bump

@timabbott
Copy link
Member

Sorry for the delay -- holidays hit. @andersk can you take over reviewing this?

@andersk
Copy link
Member

andersk commented Jan 3, 2019

@Savemech If you want to help things along, follow the guidelines that Tim linked (GitHub guide, commit guidelines). For example, there could be one commit that just does the bump to bionic (with no other changes, except any that might be required for the bionic bump), one commit that just does the tar.gz changes, and so on. This is the organization we’ll need before we can merge anything, and it’s helpful during review.

DEBIAN_FRONTEND=noninteractive could be set using ARG so it doesn’t need to be duplicated everywhere.

What was going wrong without the timezone configuration?

jameswinegar and others added 3 commits January 17, 2019 15:35
use ARG for DEBIAN_FRONTEND
* Squash second locale-gen call
* Move export call for puppet into separated ENV
@timabbott
Copy link
Member

Closing, since many of the changes are stale (we've since done them via other PRs) and others are not well separated.

Thanks for your work on this @Savemech!

@timabbott timabbott closed this Nov 23, 2022
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