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

secret coordinate helpers #6114

Merged
merged 47 commits into from
Sep 23, 2021
Merged

secret coordinate helpers #6114

merged 47 commits into from
Sep 23, 2021

Conversation

jrhizor
Copy link
Contributor

@jrhizor jrhizor commented Sep 15, 2021

resolves #6099

@github-actions github-actions bot added the area/platform issues related to the platform label Sep 16, 2021
@jrhizor jrhizor temporarily deployed to more-secrets September 17, 2021 19:05 Inactive
@jrhizor jrhizor temporarily deployed to more-secrets September 17, 2021 19:15 Inactive
@jrhizor jrhizor temporarily deployed to more-secrets September 17, 2021 20:05 Inactive
@jrhizor jrhizor temporarily deployed to more-secrets September 20, 2021 15:04 Inactive
@jrhizor jrhizor temporarily deployed to more-secrets September 20, 2021 15:20 Inactive
@jrhizor
Copy link
Contributor Author

jrhizor commented Sep 20, 2021

todo:

  • add airbyte prefix to secrets
  • clean up
  • add followup tickets
  • add docs
  • decide if we want true versions or to always bump (and implement if needed)

@jrhizor jrhizor temporarily deployed to more-secrets September 20, 2021 15:30 Inactive
@jrhizor jrhizor temporarily deployed to more-secrets September 20, 2021 23:55 Inactive
@jrhizor jrhizor marked this pull request as ready for review September 22, 2021 23:48
@jrhizor jrhizor temporarily deployed to more-secrets September 22, 2021 23:49 Inactive
Copy link
Contributor

@sherifnada sherifnada left a comment

Choose a reason for hiding this comment

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

Was mainly looking at test cases, the main one I would suggest adding is a nested oneof (inside an object)

another thing is that it would have been super helpful to have javadocs on the classes and public methods to help the reader understand what's going on.

@jrhizor jrhizor temporarily deployed to more-secrets September 23, 2021 04:13 Inactive
@jrhizor jrhizor temporarily deployed to more-secrets September 23, 2021 05:01 Inactive
@jrhizor
Copy link
Contributor Author

jrhizor commented Sep 23, 2021

@sherifnada I fixed the ArrayOneOfTestCase and added a NestedOneOfTestCase. Is this what you were talking about?

I also cleaned up the split function so that should be actually reviewable.

I'm going to add a ton of javadocs in the morning and slightly improve on some of the naming and then merge if there aren't any objections.

@jrhizor jrhizor temporarily deployed to more-secrets September 23, 2021 15:58 Inactive
@jrhizor jrhizor temporarily deployed to more-secrets September 23, 2021 19:18 Inactive
@jrhizor jrhizor temporarily deployed to more-secrets September 23, 2021 19:30 Inactive
@jrhizor jrhizor temporarily deployed to more-secrets September 23, 2021 20:31 Inactive
@jrhizor jrhizor temporarily deployed to more-secrets September 23, 2021 20:36 Inactive
@jrhizor jrhizor temporarily deployed to more-secrets September 23, 2021 20:41 Inactive
@jrhizor jrhizor temporarily deployed to more-secrets September 23, 2021 21:07 Inactive
@jrhizor jrhizor temporarily deployed to more-secrets September 23, 2021 21:10 Inactive
@jrhizor jrhizor merged commit a8261f4 into master Sep 23, 2021
@jrhizor jrhizor deleted the jrhizor/secret-coordinate-helpers branch September 23, 2021 21:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/platform issues related to the platform
Projects
None yet
Development

Successfully merging this pull request may close these issues.

define structure for partial secret configs / coordinate usage
3 participants