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

🌱 Move the internal machinery utils to appropiate exported packages #2074

Merged
merged 1 commit into from
Mar 10, 2021

Conversation

Adirio
Copy link
Contributor

@Adirio Adirio commented Mar 9, 2021

  • Go version checks and go-modules-based repo discovery moved to pkg/plugins/golang
  • Command execution and user prompt moved to pkg/plugin/util

Partial: #2036

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Mar 9, 2021
@k8s-ci-robot k8s-ci-robot requested review from droot and mengqiy March 9, 2021 12:04
@k8s-ci-robot k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Mar 9, 2021
@@ -24,6 +24,96 @@ import (
"strings"
)

const (
goVerPattern = `^go(?P<major>[0-9]+)\.(?P<minor>[0-9]+)(?:\.(?P<patch>[0-9]+)|(?P<pre>(?:alpha|beta|rc)[0-9]+))?$`
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Regular expression decomposition with comprehensive test suite: https://regex101.com/r/TxisR1/1

@Adirio Adirio mentioned this pull request Mar 9, 2021
14 tasks
Copy link
Member

@camilamacedo86 camilamacedo86 left a comment

Choose a reason for hiding this comment

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

We are making public + adding tests.
So, it will not affect other places and just bring the option for them to use it 👍

- Go version checks and go-modules-based repo discovery moved to pkg/plugins/golang
- Command execution and user prompt moved to pkg/plugin/util

Signed-off-by: Adrian Orive <[email protected]>
@Adirio Adirio force-pushed the export-machinery-utils branch from 6bb8c0f to 2687774 Compare March 10, 2021 11:20
Copy link
Member

@camilamacedo86 camilamacedo86 left a comment

Choose a reason for hiding this comment

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

It does not change the behaviour.
Only add more test and allow it to be used as agreed in the bug triage meetings.

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 10, 2021
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Adirio, camilamacedo86

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [Adirio,camilamacedo86]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot merged commit 8e0f0ab into kubernetes-sigs:master Mar 10, 2021
@Adirio Adirio deleted the export-machinery-utils branch March 10, 2021 12:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants