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

[WIP] init(dockerfile): add additional file #3

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

georgettica
Copy link

@georgettica georgettica commented Dec 30, 2020

related to Bash-it/bash-it#1753

currently fails on pypa/virtualenv#1907 if I understand correctly

to test run 'docker built -t test -f pre-commit.Dockerfile .' and see the error

currently fails on pypa/virtualenv#1907 if I understand correctly

to test run 'docker built -t test -f pre-commit.Dockerfile .' and see the error
@georgettica georgettica changed the title init(dockerfile): add additional file [WIP] init(dockerfile): add additional file Dec 30, 2020
@georgettica
Copy link
Author

@ellerbrock are you still maintaining this repo?
If so can you help me solve this issue?

Copy link
Member

@NoahGorny NoahGorny left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@georgettica nice work! I left a couple of comments, but this seems like a nice idea.

@@ -0,0 +1,61 @@
# Developer:
# ---------
# Name: Maik Ellerbrock
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wouldnt write this under Maik's name. as he did not write it, you can write it your name here, it does not bear any responsibility 😄


FROM bash:5

MAINTAINER Maik Ellerbrock
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Put my name here instead, as Maik is not really present now

#
# Description:
# -----------
# Bash Shell v.5 bats and deps for development
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as I said, lets just use the minimal dockerfile that you wrote for the pre-commit- no need for complicated stuff

@georgettica
Copy link
Author

I'll fix these issues and I am glad you reviewed it :)

Thanks

Co-authored-by: Noah Gorny <noah@gittabags.com>
@georgettica
Copy link
Author

it is 1Gi, if we still want to keep it I am all for it :)

you can review again @NoahGorny

@NoahGorny
Copy link
Member

it is 1Gi, if we still want to keep it I am all for it :)

you can review again @NoahGorny

Good job @georgettica !
I would like to have some docs about this dockerfile- we also need to make sure we know how to build it and publish it!
Please add some docs on how to build the docker as well

@NoahGorny
Copy link
Member

Hi @ellerbrock
can you please take a look at this?
if you do not have the time- can you give me write permissions instead? I also maintain bash-it at the moment

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.

2 participants