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

[BugFix: Issue #338] Skip docker login from forks #339

Merged
merged 1 commit into from
Apr 10, 2022

Conversation

dikhan
Copy link
Owner

@dikhan dikhan commented Apr 10, 2022

What problem does this Pull Request solve?

Follow-up fix for #337

  • Travis does not allow secret env variables to be used in forks which results into all the PRs opened by external contributors to fail. By adding this docker logins will not be executed and therefore builds now may still fail due to docker free quota being reached (since the egres ip from travis is shared among all the hosts). This is preferable as at least there is a change that the build will pass, unfortunately Travis does not provide any solution for this at the moment.

Screen Shot 2022-04-09 at 4 28 27 PM

Soure: https://docs.travis-ci.com/user/environment-variables/

Please link to the issue number here (issue will be closed when PR is merged): Closes #338

If you don't have an issue created, please create the issue first describing the problem and then open the PR

Type of change

What type of change does your code introduce to the provider? Please put an x (w/o heading/trailing white spaces) in the boxes that apply:

  • New feature (change that adds new functionality)
  • Bug-fix (change that fixes current functionality)
  • Tech debt (enhances the current functionality)
  • New release (pumps the version)

Checklist

Please put an x (w/o heading/trailing white spaces) in the boxes that apply:

  • I have read and followed the CONTRIBUTING guidelines
  • I have added tests that prove my fix is effective or that my feature works
  • I have made sure code compiles correctly and all tests are passing by running make test-all
  • I have added/updated necessary documentation (if appropriate)
  • I have added the following info to the title of the PR (pick the appropriate option for the type of change). This is important because the release notes will include this information.
    • Feature Request: PRs related to feature requests should have in the title [FeatureRequest: Issue #X] <PR Title>
    • Bug Fixes: PRs related to bug fixes should have in the title [BugFix: Issue #X] <PR Title>
    • Tech Debt: PRs related to technical debt should have in the title [TechDebt: Issue #X] <PR Title>
    • New Release: PRs related to a new release should have in the title [NewRelease] vX.Y.Z

Checklist for Admins

  • Label is populated
  • PR is assigned to the corresponding project
  • PR has at least 1 reviewer and 1 assignee

@dikhan dikhan self-assigned this Apr 10, 2022
- Travis does not allow secret env variables to be used in forks which results into all the PRs opened by external contributors to fail. By adding this docker logins will not be executed and therefore builds now may still fail due to docker free quota being reached (since the egres ip from travis is shared among all the hosts). This is preferable as at least there is a change that the build will pass, unfortunately Travis does not provide any solution for this at the moment.
@dikhan dikhan force-pushed the skip-docker-login-on-forks branch from be1292a to 50d0e52 Compare April 10, 2022 04:23
@codecov
Copy link

codecov bot commented Apr 10, 2022

Codecov Report

Merging #339 (50d0e52) into master (b648612) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #339   +/-   ##
=======================================
  Coverage   93.66%   93.66%           
=======================================
  Files          60       60           
  Lines        3584     3584           
=======================================
  Hits         3357     3357           
  Misses        160      160           
  Partials       67       67           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b648612...50d0e52. Read the comment docs.

@dikhan dikhan added the bug-fix label Apr 10, 2022
@dikhan dikhan merged commit 0b9b746 into master Apr 10, 2022
@dikhan dikhan deleted the skip-docker-login-on-forks branch April 10, 2022 04:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Docker logins don't work on forks
1 participant