Skip to content

Conversation

@guicassolato
Copy link
Contributor

Due to lua_modules make target invoking rover lock command before translate_git_protocol (see #1320), commonly used targets like dependencies often fail with a Connection refused error.

Reported in #1375.

@guicassolato guicassolato requested a review from a team as a code owner November 9, 2022 09:37
Copy link
Member

@eguzki eguzki left a comment

Choose a reason for hiding this comment

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

I do not think that the .gitconfig is needed because the translate_git_protocol adds the config globally inside the container, but does not do any harm.

@guicassolato
Copy link
Contributor Author

I do not think that the .gitconfig is needed because the translate_git_protocol adds the config globally inside the container, but does not do any harm.

Adding globally inside the container is exactly the problem here. Since the git context for global is the project directory itself, git config --global adds the .gitconfig file to the changes. Even after you leave the container, you still see that file uncommitted.

We could try .gitignoring .gitconfig instead if you like.

@eguzki
Copy link
Member

eguzki commented Nov 9, 2022

nce the git conte

Ok I see, $HOME is the APIcast working dir. So, what I would do is to add the .gitconfig to the .gitignore and we do not need to commit it, as the content is duplicated in the Makefile. Not a big deal, as you want. The PR is already approved.

Due to `lua_modules` make target invoking `rover lock` command before `translate_git_protocol` (see #1320), commonly used targets like `dependencies` often fail with a `Connection refused` error.

Reported in #1375.
@eguzki eguzki merged commit a4f1123 into 3scale:master Nov 9, 2022
@guicassolato guicassolato deleted the fix/use-git-https-protocol branch November 9, 2022 13:35
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