Skip to content

Conversation

stilvoid
Copy link
Contributor

The current Dockerfile does not install groff and doesn't follow Dockerfile best practice.

This builds a fully working container. I've set up an automated build at https://hub.docker.com/r/stilvoid/saws/.

@codecov-io
Copy link

Current coverage is 94.52%

Merging #75 into master will not affect coverage as of 3de77e5

@@            master     #75   diff @@
======================================
  Files           29      29       
  Stmts         1241    1241       
  Branches         0       0       
  Methods          0       0       
======================================
  Hit           1173    1173       
  Partial          0       0       
  Missed          68      68       

Review entire Coverage Diff as of 3de77e5

Powered by Codecov. Updated on successful CI builds.

@cliveza
Copy link
Contributor

cliveza commented Dec 10, 2015

I submitted a follow up pull request to fix my error, but @stilvoid dockerfile is better, removing mine.

@stilvoid
Copy link
Contributor Author

Thanks @cliveza. I looked for issues or pull requests related to this problem but couldn't find any.

donnemartin added a commit that referenced this pull request Dec 11, 2015
Fix installation of groff and follow Dockerfile best practices
@donnemartin donnemartin merged commit 48f6761 into donnemartin:master Dec 11, 2015
@donnemartin
Copy link
Owner

Thanks @stilvoid @cliveza!

@stilvoid
Copy link
Contributor Author

Strangely, I don't see these changes in the master branch any more. Has there been a force push since?

@donnemartin
Copy link
Owner

@stilvoid @cliveza DOH! I was trying a few things in the develop branch and intended to blow away just those changes. Looks like I accidentally toppled your changes in master, sorry!

I've hand merged the pull requests (it was a little messy) but we should be all set now.

To prevent similar issues in the future I set up branch protection. Actually, I went ahead and set up branch protection for all my other repos too.

Apologies for the inconvenience, and thanks for catching it.

@stilvoid
Copy link
Contributor Author

No worries. I'd have happily rebased and resubmitted my PR but this works just fine :)

I'll update my fork so the automated build here is up to date.

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