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.

Sign up for GitHub

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

Jump to bottom

Include PROJECT file information into model.Universe #1313

Merged
merged 1 commit into from
Jan 28, 2020
Merged

Include PROJECT file information into model.Universe #1313

merged 1 commit into from
Jan 28, 2020

Conversation

Adirio
Copy link
Contributor

@Adirio Adirio commented Jan 16, 2020 •
edited
Loading

Description

Include the PROJECT file into model.Universe.

It also provides a constructor for model.Universe called model.NewUniverse that uses the functional options paradigm to provide optional configuration.

Motivation

The main motivation is providing plugins access to the configuration file. Additionally, scaffold.Execute needs some information aside from the files it scaffolds. Right now this informnation is provided in three different ways. As fields to the Scaffold instance, the first parameter to Scaffold.Execute (model.Universe), and the second parameter to Scaffold.Execute (input.Options). This change is a first step into providing a single way to provide that information.

This PR is part of a bigger change tracked in #1218 but can be applied rightaway.

/kind feature

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/feature Categorizes issue or PR as related to a new feature. labels Jan 16, 2020
@k8s-ci-robot
Copy link
Contributor

Hi @Adirio. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@k8s-ci-robot k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jan 16, 2020
@Adirio

This comment has been minimized.

Sign in to view
@Adirio Adirio closed this Jan 16, 2020
@Adirio Adirio reopened this Jan 16, 2020
@Adirio Adirio closed this Jan 16, 2020
@Adirio Adirio reopened this Jan 16, 2020
@Adirio Adirio force-pushed the scaffold-enhancement/project-in-universe branch from 080862c to 13a2c6f Compare January 16, 2020 15:28
camilamacedo86
camilamacedo86 reviewed Jan 17, 2020
View reviewed changes
pkg/scaffold/api.go Outdated Show resolved Hide resolved
camilamacedo86
camilamacedo86 reviewed Jan 17, 2020
View reviewed changes
pkg/scaffold/api.go Outdated Show resolved Hide resolved
camilamacedo86
camilamacedo86 reviewed Jan 17, 2020
View reviewed changes
pkg/scaffold/api.go Outdated Show resolved Hide resolved
@Adirio Adirio force-pushed the scaffold-enhancement/project-in-universe branch from 13a2c6f to 96d3016 Compare January 17, 2020 10:25
@Adirio Adirio force-pushed the scaffold-enhancement/project-in-universe branch 2 times, most recently from 98a6277 to 6927a52 Compare January 17, 2020 12:10
@Adirio Adirio mentioned this pull request Jan 17, 2020
Closed
@Adirio Adirio force-pushed the scaffold-enhancement/project-in-universe branch 2 times, most recently from cd844ae to bebd928 Compare January 20, 2020 09:13
@Adirio
Copy link
Contributor Author

Adirio commented Jan 20, 2020

All the suggestions were tackled. Could we move forward?

@DirectXMan12
Copy link
Contributor

@Adirio sure, I'm on board. Might want to rebase on #1325 since it should be merging soonish. Ping me on slack if things don't get merged

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 27, 2020
@Adirio Adirio force-pushed the scaffold-enhancement/project-in-universe branch from bebd928 to e48e98b Compare January 27, 2020 18:53
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Jan 27, 2020
@Adirio
Copy link
Contributor Author

Adirio commented Jan 27, 2020

Rebased, there were a lot of overlaping sections so I may have missed something, so I'm force-pushing now while I do a final check to make Travis also check it meanwhile.

@Adirio Adirio force-pushed the scaffold-enhancement/project-in-universe branch from e48e98b to e9bd5b9 Compare January 27, 2020 19:04
estroz
estroz approved these changes Jan 27, 2020
View reviewed changes
Copy link
Contributor

@estroz estroz left a comment

Choose a reason for hiding this comment

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

LGTM other than a few error message nits.

pkg/scaffold/api.go Outdated Show resolved Hide resolved
pkg/scaffold/api.go Outdated Show resolved Hide resolved
pkg/scaffold/api.go Outdated Show resolved Hide resolved
pkg/scaffold/api.go Outdated Show resolved Hide resolved
pkg/scaffold/api.go Outdated Show resolved Hide resolved
@Adirio Adirio closed this Jan 27, 2020
@Adirio Adirio reopened this Jan 27, 2020
@Adirio Adirio force-pushed the scaffold-enhancement/project-in-universe branch 3 times, most recently from 0194333 to 6534660 Compare January 28, 2020 09:06
@Adirio
Include PROJECT file information into model.Universe
6534660 
Signed-off-by: Adrian Orive <adrian.orive.oneca@gmail.com>
@Adirio Adirio requested a review from estroz January 28, 2020 14:45
@Adirio
Copy link
Contributor Author

Adirio commented Jan 28, 2020

@DirectXMan12 The rebase should be ready

@DirectXMan12
Copy link
Contributor

/lgtm
/approve

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

[APPROVALNOTIFIER] This PR is APPROVED

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

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:

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 added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 28, 2020
@k8s-ci-robot k8s-ci-robot merged commit dc5ea0c into kubernetes-sigs:master Jan 28, 2020
@Adirio Adirio deleted the scaffold-enhancement/project-in-universe branch January 28, 2020 18:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@camilamacedo86 camilamacedo86 camilamacedo86 left review comments

@droot droot Awaiting requested review from droot

@estroz estroz Awaiting requested review from estroz

Assignees

@DirectXMan12 DirectXMan12

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. kind/feature Categorizes issue or PR as related to a new feature. lgtm "Looks good to me", indicates that a PR is ready to be merged. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@Adirio @k8s-ci-robot @DirectXMan12 @camilamacedo86 @estroz