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

Dockerfile and some fixes for ECR #7

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

Conversation

tomaszkiewicz
Copy link
Contributor

Hi,

As I've promised I'm opening a pull request with the rest of my changes, I mean:

  1. I've added Dockerfile to build a project into docker image
  2. I've changed the logic of parsing repo name - instead of depending on some prefix, now we use full url, that contains region - it is very useful to do that this way, because we have unified way of addressing and also we don't have to have any ~/.aws/config in place to set a region (which is a kind of trouble in docker scenarios)
  3. Fixed manifest for destination - ECR requires to set manifest name to be the same as repository name.

The change number 2 is not backward compatible.

I understand that you can reject this PR to be backward compatible and it's ok :)

Best regards

Łukasz Tomaszkiewicz

@mdlavin
Copy link
Owner

mdlavin commented Oct 3, 2018

I've been horrible for not responding faster. How would you feel about taking over the project? I should be able to give you permission to merge and I can give you access to publish on npm too. Would you be interested in that?

@tomaszkiewicz
Copy link
Contributor Author

I've encountered some issues related to my fixes of manifest - it looks like in some cases docker refuses to use the image as the signature is invalid, unfortunately I haven't figured out the solution yet :(
After I resolve the issue I think I can take over the project if you wish :)

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