Skip to content
This repository was archived by the owner on Oct 30, 2024. It is now read-only.

use architect-orb v0.8.8#408

Merged
tfussell merged 19 commits intomasterfrom
update-architect-orb
Apr 16, 2020
Merged

use architect-orb v0.8.8#408
tfussell merged 19 commits intomasterfrom
update-architect-orb

Conversation

@tfussell
Copy link
Contributor

@tfussell tfussell commented Apr 13, 2020

Checklist

  • Consider SIG UX feedback.
  • Update changelog in CHANGELOG.md.

@tfussell tfussell self-assigned this Apr 13, 2020

var scheme = runtime.NewScheme()
var codecs = serializer.NewCodecFactory(scheme)
-var parameterCodec = runtime.NewParameterCodec(scheme)
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd rather replace that with var parameterCodec = runtime.NewParameterCodec(scheme) // nolint just in case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

Comment on lines +138 to +140
StateActive ReleaseState = "active"
StateDeprecated ReleaseState = "deprecated"
StateWIP ReleaseState = "wip"
Copy link
Contributor

Choose a reason for hiding this comment

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

Is exporting this a legit part of this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The linter didn't like the unexported variables. I guess I could add a // nolint comment here, but it also seems reasonable to export them. Not sure.

Copy link
Contributor

@kopiczko kopiczko Apr 14, 2020

Choose a reason for hiding this comment

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

Linter didn't like unexported?! I'd prefer // nolint but I'm not big on that here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I made these unexported again based on your suggestion.

@tfussell tfussell marked this pull request as ready for review April 16, 2020 02:52
@tfussell tfussell requested a review from a team April 16, 2020 02:52
@tfussell tfussell changed the base branch from replace-deepcopytime to master April 16, 2020 02:53
@tfussell
Copy link
Contributor Author

Comments addressed. Going with this.

@tfussell tfussell merged commit 9493ccf into master Apr 16, 2020
@tfussell tfussell deleted the update-architect-orb branch April 16, 2020 03:24
teemow added a commit that referenced this pull request Apr 16, 2020
* master:
  use architect-orb v0.8.8 (#408)
  Replace DeepCopyTime with metav1.Time (#407)
  Add missing releases to changelog (#411)
  Document G8sControlPlane CRD (#405)
  Add Chart CR (#406)
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.

2 participants