Skip to content
This repository has been archived by the owner on Mar 11, 2021. It is now read-only.

Use golang 1.11 from epel #2378

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

Conversation

jarifibrahim
Copy link
Member

@jarifibrahim jarifibrahim commented Dec 13, 2018

@codecov
Copy link

codecov bot commented Dec 13, 2018

Codecov Report

❗ No coverage uploaded for pull request base (master@874db20). Click here to learn what that means.
The diff coverage is 25%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #2378   +/-   ##
=========================================
  Coverage          ?   70.11%           
=========================================
  Files             ?      174           
  Lines             ?    17121           
  Branches          ?        0           
=========================================
  Hits              ?    12005           
  Misses            ?     3951           
  Partials          ?     1165
Impacted Files Coverage Δ
auth/authz.go 0% <0%> (ø)
controller/deployments.go 55.84% <0%> (ø)
login/service.go 10.66% <0%> (ø)
controller/work_item_link.go 81.74% <100%> (ø)

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 874db20...485b6d4. Read the comment docs.

@jarifibrahim jarifibrahim changed the title WIP Use golang 1.11 from epel Use golang 1.11 from epel Dec 13, 2018
Copy link
Collaborator

@michaelkleinhenz michaelkleinhenz left a comment

Choose a reason for hiding this comment

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

LGTM

Dockerfile.builder Outdated Show resolved Hide resolved
Dockerfile.builder Outdated Show resolved Hide resolved
Dockerfile.builder Show resolved Hide resolved
findutils \
git \
$(test "$USE_GO_VERSION_FROM_WEBSITE" != 1 && echo "golang") \
golang \
Copy link
Collaborator

Choose a reason for hiding this comment

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

I highly object against this change. I explicitly want to be able to use Go from the website in order to test out new features in builds like the coverage for example. If you find that CentOS has ditched Golang as it appears, then please fix that problem and don't just remove the ability to use Go from the website.

Copy link
Member Author

Choose a reason for hiding this comment

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

I highly object against this change. I explicitly want to be able to use Go from the website in order to test out new features in builds like the coverage for example.

We can still test things via a PR, can't we? Even if we keep the flag, you'd still have to send a new PR if you want to test something :)

If you find that CentOS has ditched Golang as it appears, then please fix that problem and don't just remove the ability to use Go from the website.

Using golang from epel is exactly what we want. I wasn't part of the discussion where it was decided to use golang from epel and not website but it looks like most of our services have already migrated openshiftio/openshift.io#4618

Copy link
Collaborator

Choose a reason for hiding this comment

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

By testing I mean: Permanently use a feature (not only in one PR).

I'm not suggesting to not use Go from epel. I just want you to not remove the feature to be able to use Go from the website.

Copy link
Member Author

Choose a reason for hiding this comment

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

@kwk okay. That makes sense. I'll undo the removal of from-website flag.

cico_run_coverage.sh Show resolved Hide resolved
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants