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

Makes popper independent of .git #664

Merged
merged 2 commits into from
Jun 6, 2019

Conversation

JayjeetAtGithub
Copy link
Collaborator

@JayjeetAtGithub JayjeetAtGithub commented May 30, 2019

Addresses #665, #647

@pep8speaks
Copy link

pep8speaks commented May 30, 2019

Hello @JayjeetAtGithub! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2019-06-04 09:17:26 UTC

@JayjeetAtGithub
Copy link
Collaborator Author

@ivotron When we return "" from scm.get_sha() method, then in this line in gha.py, the tag variable in docker runner remains incomplete and throws error due to self.env['GITHUB_SHA'] being empty. With what should we replace the return value to handle this situation ?

cli/popper/gha.py Outdated Show resolved Hide resolved
@JayjeetAtGithub
Copy link
Collaborator Author

JayjeetAtGithub commented May 31, 2019

@ivotron Can you please tell me what would be the best way to run all the tests with and without .git ?
I have achieved it by setting an env variable POPPER_TEST_MODE and changing the init_test_repo body in ci/test/common depending on the env vars value.

@JayjeetAtGithub
Copy link
Collaborator Author

@ivotron Should we fail or warn on finding a detached head? Means I want to know whether it is ok to have a detached head. Currently, I check for a detached head in init_repo_object in scm.py and fail if one is found. This line puts the head in detached state and build fails. How to deal with this ? Thanks

@JayjeetAtGithub JayjeetAtGithub marked this pull request as ready for review June 2, 2019 14:11
.travis.yml Outdated Show resolved Hide resolved
.travis.yml Outdated Show resolved Hide resolved
ci/test/dry-run Outdated Show resolved Hide resolved
cli/popper/scm.py Outdated Show resolved Hide resolved
@JayjeetAtGithub JayjeetAtGithub force-pushed the jayjeet/issue647 branch 2 times, most recently from 64a70e7 to 118fb13 Compare June 3, 2019 14:40
@JayjeetAtGithub
Copy link
Collaborator Author

@ivotron Made the changes. I called check_if_detached_head from scm.py in CI env because if I would call it through gha.py, I would naturally skip the check when using the scm functions separately, like in the get_head_commit(), etc and run into error.

@ivotron ivotron merged commit 8a5fbcb into getpopper:master Jun 6, 2019
@ivotron
Copy link
Collaborator

ivotron commented Jun 6, 2019

thanks a lot!

@JayjeetAtGithub JayjeetAtGithub deleted the jayjeet/issue647 branch August 7, 2019 14:01
ivotron pushed a commit that referenced this pull request May 24, 2020
Makes popper independent of having a .git repo initalized

fixes #665 
fixes #647
ivotron pushed a commit that referenced this pull request May 25, 2020
Makes popper independent of having a .git repo initalized

fixes #665 
fixes #647
ivotron pushed a commit that referenced this pull request May 25, 2020
Makes popper independent of having a .git repo initalized

fixes #665 
fixes #647
ivotron pushed a commit that referenced this pull request May 25, 2020
Makes popper independent of having a .git repo initalized

fixes #665 
fixes #647
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.

3 participants