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 Secret Generation Strategies extension #161

Merged

Conversation

baijum
Copy link
Contributor

@baijum baijum commented Jun 10, 2021

Based on the discussion in #158 and #156, this extension can become a
separate standard by itself. Move the extension into a separate file.

@scothis
Copy link
Contributor

scothis commented Jun 10, 2021

Since this is our last "extension" should we also remove the whole section?

Copy link
Member

@nebhale nebhale left a comment

Choose a reason for hiding this comment

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

Also, let's make sure there's a concrete issue opened to ensure we don't lose the functionality, at least in an implementation and possibly as a separate spec.

README.md Outdated
@@ -628,218 +623,3 @@ rules: [] # The control plane automatically fills in the rules
# Extensions
Copy link
Member

Choose a reason for hiding this comment

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

Let's remove the entire Extensions section.

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. 1c03564

baijum added a commit to secret-generator/spec that referenced this pull request Jun 10, 2021
@baijum
Copy link
Contributor Author

baijum commented Jun 10, 2021

Also, let's make sure there's a concrete issue opened to ensure we don't lose the functionality, at least in an implementation and possibly as a separate spec.

I have added the current extension text verbatim to this repository:
https://github.com/secret-generator/spec

@baijum baijum changed the title Remove Secret Generation Strategies extension Move Secret Generation Strategies extension Jun 10, 2021
@baijum
Copy link
Contributor Author

baijum commented Jun 10, 2021

Also, let's make sure there's a concrete issue opened to ensure we don't lose the functionality, at least in an implementation and possibly as a separate spec.

I have added the current extension text verbatim to this repository:
https://github.com/secret-generator/spec

Based on today's call. I have moved the extension into a separate file.

@@ -0,0 +1,214 @@
# Secret Generator Extension for Service Binding
Copy link
Contributor

Choose a reason for hiding this comment

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

it needs to be clear that this content is not a required part of the spec

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can add a note. I need some help with phrasing it. Here is my attempt:

Note: This spec is dependent on "Service Binding Specification for Kubernetes", but there is no direct association -- neither containment nor aggregation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

One more sentence pointing to the release cycle:

Note: This spec is dependent on "Service Binding Specification for Kubernetes", but there is no direct association -- neither containment nor aggregation. Both specs are going to have their independent release cycles.

Copy link
Contributor

Choose a reason for hiding this comment

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

What if we moved it into a directory named "extensions" and added a README.md that contained this paragraph?

Copy link
Contributor

Choose a reason for hiding this comment

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

By and large, this is the issue with keeping placeholder content in the repo. It becomes murky as to what it means and why it exists.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  • Created an "extensions" directory and moved the Secret Generation extension inside
  • Created "extensions/README.md" with a brief introduction and a table of extensions (No., Title, and Status columns)
  • Updated the extension with a brief introduction and status section

Based on the discussion in servicebinding#158 and servicebinding#156, this extension can become a
separate standard by itself.
It can be added back again once there is at least one extension.
@baijum baijum force-pushed the remove-secret-generation-extension branch from 3652b26 to edbad80 Compare June 17, 2021 14:54
@baijum
Copy link
Contributor Author

baijum commented Jun 25, 2021

Notes from yesterday's meeting:

  • Evolved to a separate file
  • Still needs a full review of content
  • Think about how to display sub-specs (CloudEvents)

@baijum
Copy link
Contributor Author

baijum commented Jun 25, 2021

  • Think about how to display sub-specs (CloudEvents)

I have created an extensions list in the extensions/README.md

Copy link
Contributor

@scothis scothis left a comment

Choose a reason for hiding this comment

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

The move looks good, I'm not in a place to judge the content of the extension itself

Copy link
Member

@arthurdm arthurdm left a comment

Choose a reason for hiding this comment

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

LGTM

@nebhale nebhale merged commit 9f0971e into servicebinding:master Aug 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants