10000 cli: change generate-config flag to update-config flag by miampf · Pull Request #1897 · edgelesssys/constellation · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

cli: change generate-config flag to update-config flag #1897

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
Merged
merged 2 commits into from
Jun 28, 2023
Merged

cli: change generate-config flag to update-config flag #1897

merged 2 commits into from
Jun 28, 2023

Conversation

miampf
Copy link
Contributor
@miampf miampf commented Jun 7, 2023 •
edited by thomasten
Loading

Proposed change(s)

  • change --generate-config flag to --update-config in the iam create subcommand
  • rename --write-config flag to --update-config in the upgrade check subcommand

Context

SInce we'd need to support the growing number of flags in constellation config generate for the --generate-config flag, we should change the workflow to scale better while preserving flexibility.

Additional info

  • This changes the workflow to require constellation config generate before constellation iam create

Checklist

  • Update docs
  • Add labels (e.g., for changelog category)
  • Link to Milestone

@miampf miampf added bug fix Fixing a bug no changelog Change won't be listed in release changelog labels Jun 7, 2023
@miampf miampf added this to the v2.9.0 milestone Jun 7, 2023
@miampf miampf changed the title Ref/update config flag refactor: change generate-config flag to update-config flag Jun 7, 2023
@netlify
Copy link
netlify bot commented Jun 7, 2023 •
edited
Loading

✅ Deploy Preview for constellation-docs ready!

Name Link
🔨 Latest commit 0ea1d71
🔍 Latest deploy log https://app.netlify.com/sites/constellation-docs/deploys/649c28c7e8f2870008527062
😎 Deploy Preview https://deploy-preview-1897--constellation-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

derpsteb
derpsteb reviewed Jun 8, 2023
View reviewed changes
thomasten
thomasten re 10000 quested changes Jun 9, 2023
View reviewed changes
Copy link
Member
@thomasten thomasten left a comment

Choose a reason for hiding this comment

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

Thanks, changes mostly look good. Some small suggestions and some things missing.

@miampf miampf force-pushed the ref/update-config-flag branch from a1dfc7f to 79d16fd Compare June 12, 2023 12:24
@msanft msanft mentioned this pull request Jun 13, 2023
Merged
3 tasks
@elchead elchead self-requested a review June 14, 2023 09:36
@derpsteb
Copy link
Contributor

Removing bugfix label as this does not fix a bug in an existing release and should not be listed as bugfix in the changelog.

@derpsteb derpsteb removed the bug fix Fixing a bug label Jun 14, 2023
@katexochen katexochen removed their request for review June 15, 2023 12:57
@miampf miampf force-pushed the ref/update-config-flag branch from 4b2e85b to b8ba69b Compare June 21, 2023 13:35
@miampf miampf requested review from derpsteb and thomasten June 21, 2023 15:01
@elchead
Copy link
Contributor
elchead commented Jun 21, 2023 •
edited
Loading

This change should definetly appear in the changelog. Under breaking-change @derpsteb ?

Please prepend a Context headline in the PR (as per new template) where you put the reasoning of additional info (for the release changelog). It's noteworthy to add that config generate was not required before iam create when adding --generate-config if I'm not mistaken.

I think the biggest gain is the better user flow. I.e. the user can use an existing config and add the credentials when doing iam create ?

@elchead
Copy link
Contributor
elchead commented Jun 21, 2023 •
edited
Loading

Please update all occurences of --generate-config in the code + doc

elchead
elchead reviewed Jun 21, 2023
View reviewed changes
@miampf
Copy link
Contributor Author
miampf commented Jun 21, 2023

Please update all occurences of --generate-config in the code + doc

I believe everything is updated now? The occurrences in the docs are all under versioned_docs so I don't believe I should update them.

elchead
elchead reviewed Jun 21, 2023
View reviewed changes
elchead
elchead reviewed Jun 21, 2023
View reviewed changes
elchead
elchead reviewed Jun 21, 2023
View reviewed changes
elchead
elchead reviewed Jun 21, 2023
View reviewed changes
elchead
elchead reviewed Jun 21, 2023
View reviewed changes
@elchead
Copy link
Contributor
elchead commented Jun 21, 2023

thanks for tackling this. lgtm with changes

@thomasten thomasten changed the title refactor: change generate-config flag to update-config flag cli: change generate-config flag to update-config flag Jun 22, 2023
@thomasten thomasten removed the no changelog Change won't be listed in release changelog label Jun 22, 2023
thomasten
thomasten reviewed Jun 22, 2023
View reviewed changes
Copy link
Member
@thomasten thomasten left a comment

Choose a reason for hiding this comment

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

some last style nits, otherwise lgtm now

@derpsteb
Copy link
Contributor
derpsteb commented Jun 23, 2023 •
edited
Loading

For visibilty: this needs to be incl F438 uded in 2.9, otherwise we would have to adapt iam create to have a --attestation flag for AWS SNP support.

@derpsteb derpsteb added the breaking change Change breaks existing API or configuration. label Jun 23, 2023
@miampf miampf requested review from thomasten and elchead June 26, 2023 12:44
elchead
elchead reviewed Jun 26, 2023
View reviewed changes
elchead
elchead reviewed Jun 26, 2023
View reviewed changes
@elchead
Copy link
Contributor
elchead commented Jun 26, 2023

lgtm.
little nit: do you know git amend and squash? it can help to keep the git history clean :) https://blog.sebastian-daschner.com/entries/git-commit-fixup-autosquash

@miampf miampf requested a review from elchead June 28, 2023 11:52
@miampf
Copy link
Contributor Author
miampf commented Jun 28, 2023

lgtm. little nit: do you know git amend and squash? it can help to keep the git history clean :) https://blog.sebastian-daschner.com/entries/git-commit-fixup-autosquash

Thanks for the recommendation! Do you want me to try and fixup the history or do you just want to use the squash and merge function once the branch is ready since most of the commits were there to fix nits?

@elchead
Copy link
Contributor
elchead commented Jun 28, 2023

lgtm. little nit: do you know git amend and squash? it can help to keep the git history clean :) https://blog.sebastian-daschner.com/entries/git-commit-fixup-autosquash

Thanks for the recommendation! Do you want me to try and fixup the history or do you just want to use the squash and merge function once the branch is ready since most of the commits were there to fix nits?

no need to fix, since merge auto-squashes :) just for next time, so that reviewing might be a bit easier

@miampf miampf force-pushed the ref/update-config-flag branch from 8e51f79 to ab079b2 Compare June 28, 2023 12:31
miampf added 2 commits June 28, 2023 14:32
@miampf
renamed flag
bb5c732
@miampf
Squashed commit of the following:
0ea1d71 
commit 8e51f79
Author: miampf <miampf@proton.me>
Date:   Wed Jun 28 13:48:28 2023 +0200

    added dots and spaces in docs

commit fb4ef15
Author: miampf <miampf@proton.me>
Date:   Mon Jun 26 14:43:47 2023 +0200

    update configure-cluster.expect

commit 2484e05
Author: miampf <miampf@proton.me>
Date:   Mon Jun 26 14:42:20 2023 +0200

    update workflows/config.me

commit d10b7a2
Author: miampf <miampf@proton.me>
Date:   Mon Jun 26 14:37:52 2023 +0200

    shorten first steps

commit a899e24
Author: miampf <miampf@proton.me>
Date:   Mon Jun 26 14:33:51 2023 +0200

    write-config -> update-config in string

commit 60ee0a6
Author: miampf <miampf@proton.me>
Date:   Mon Jun 26 14:28:27 2023 +0200

    some more unit test renaming

commit b05a7a6
Author: miampf <miampf@proton.me>
Date:   Mon Jun 26 14:24:39 2023 +0200

    renamed unit tests

commit c00ded7
Author: miampf <miampf@proton.me>
Date:   Mon Jun 26 13:38:35 2023 +0200

    wrap error instead of printing

commit 54f717d
Author: miampf <miampf@proton.me>
Date:   Wed Jun 21 18:06:33 2023 +0200

    bazel tidy again because vscode is funky

commit 8f768a2
Author: miampf <miampf@proton.me>
Date:   Wed Jun 21 18:05:28 2023 +0200

    added unit tests and renamed symbol to `updateConfigFlag`

commit 99bf226
Author: miampf <miampf@proton.me>
Date:   Wed Jun 21 17:34:43 2023 +0200

    moved info log out of `yesFlag` if-block

commit 4c9c93c
Author: miampf <miampf@proton.me>
Date:   Wed Jun 21 17:29:10 2023 +0200

    updated forgotten files

commit 1d4b05d
Author: miampf <miampf@proton.me>
Date:   Wed Jun 21 16:55:27 2023 +0200

    bazel tidy

commit 17a2abc
Author: miampf <miampf@proton.me>
Date:   Wed Jun 21 16:12:24 2023 +0200

    make the comment a comment

commit b8ba69b
Author: miampf <miampf@proton.me>
Date:   Wed Jun 21 15:25:52 2023 +0200

    wrote expect file

commit 103bfc0
Author: miampf <miampf@proton.me>
Date:   Wed Jun 14 11:01:42 2023 +0200

    rewrote config workflow doc

commit 290c615
Author: miampf <miampf@proton.me>
Date:   Wed Jun 14 10:58:17 2023 +0200

    updated CI again

commit a0c08c5
Author: miampf <miampf@proton.me>
Date:   Wed Jun 14 10:46:18 2023 +0200

    updated missed doc and additional rfc

commit 2bd387e
Author: miampf <miampf@proton.me>
Date:   Mon Jun 12 14:28:40 2023 +0200

    modified constellation iam create CI

commit 61b8d78
Author: miampf <miampf@proton.me>
Date:   Mon Jun 12 14:23:35 2023 +0200

    bazel generate

commit 84ec2f4
Author: miampf <miampf@proton.me>
Date:   Mon Jun 12 13:52:36 2023 +0200

    moved config and iam config into points 1 and 2 of docs

commit 67c7f95
Author: miampf <miampf@proton.me>
Date:   Mon Jun 12 13:39:50 2023 +0200

    same tab groupId

commit 8a04311
Author: miampf <miampf@proton.me>
Date:   Mon Jun 12 13:32:10 2023 +0200

    existingFiles -> existingConfigFiles

commit c75d278
Author: miampf <miampf@proton.me>
Date:   Mon Jun 12 13:29:09 2023 +0200

    edited checkWorkingDir comment

commit 770e4b4
Author: miampf <miampf@proton.me>
Date:   Mon Jun 12 13:27:37 2023 +0200

    moved debug message in actual if block

commit 7524494
Author: miampf <miampf@proton.me>
Date:   Mon Jun 12 13:26:38 2023 +0200

    moved starting the spinner before actual create call

commit d85b32c
Author: miampf <miampf@proton.me>
Date:   Mon Jun 12 13:24:23 2023 +0200

    removed kubernetes flag and edited tests accordingly

commit 219dedd
Author: miampf <miampf@proton.me>
Date:   Wed Jun 7 18:45:25 2023 +0200

    removed unused parameter from `checkWorkingDir()`

commit 4166e3e
Author: miampf <miampf@proton.me>
Date:   Wed Jun 7 18:43:37 2023 +0200

    fixed spelling error

commit 06e7ec9
Author: miampf <miampf@proton.me>
Date:   Wed Jun 7 18:13:10 2023 +0200

    bazel generate

commit 829baea
Author: miampf <miampf@proton.me>
Date:   Wed Jun 7 18:03:09 2023 +0200

    brought back deleted aws unit tests which were labeled for azure

commit 963d35a
Author: miampf <miampf@proton.me>
Date:   Wed Jun 7 17:57:32 2023 +0200

    reworded generated -> updated

commit b33297c
Author: miampf <miampf@proton.me>
Date:   Wed Jun 7 17:42:58 2023 +0200

    edited documentation

commit 1d00459
Author: miampf <miampf@proton.me>
Date:   Wed Jun 7 17:10:31 2023 +0200

    possible grammar error, changed it to be sure

commit 4474593
Author: miampf <miampf@proton.me>
Date:   Wed Jun 7 17:08:53 2023 +0200

    added an explaining message above the error

commit 23c0c73
Author: miampf <miampf@proton.me>
Date:   Wed Jun 7 16:52:44 2023 +0200

    renamed write-config flag to update-config

commit c3b3e67
Author: miampf <miampf@proton.me>
Date:   Wed Jun 7 16:47:13 2023 +0200

    removed unneccessary createConfig

commit c92dd73
Author: miampf <miampf@proton.me>
Date:   Wed Jun 7 16:21:19 2023 +0200

    cleaned up azure unit test

commit e35db65
Author: miampf <miampf@proton.me>
Date:   Wed Jun 7 16:19:10 2023 +0200

    rewrote GCP unit test

commit 7454bcc
Author: miampf <miampf@proton.me>
Date:   Wed Jun 7 16:16:01 2023 +0200

    rewrote azure unit test

commit 551543c
Author: miampf <miampf@proton.me>
Date:   Wed Jun 7 16:10:00 2023 +0200

    fixed panic

commit e9ecfe7
Author: miampf <miampf@proton.me>
Date:   Mon Jun 5 15:37:56 2023 +0200

    only read from config if update-config is set

commit 1e5f320
Author: miampf <miampf@proton.me>
Date:   Mon Jun 5 15:33:22 2023 +0200

    generate valid yaml in unit test

commit b3df944
Author: miampf <miampf@proton.me>
Date:   Mon Jun 5 14:54:46 2023 +0200

    removed check for config file existing

commit d96f2bd
Author: miampf <miampf@proton.me>
Date:   Mon Jun 5 14:51:19 2023 +0200

    edited test cases, dont work yet

commit 5f57a3d
Author: miampf <miampf@proton.me>
Date:   Mon Jun 5 13:50:53 2023 +0200

    renamed more references; overwrite config instead of creating
@miampf miampf force-pushed the ref/update-config-flag branch from ab079b2 to 0ea1d71 Compare June 28, 2023 12:34
@miampf miampf merged commit 77b28cb into main Jun 28, 2023
@miampf miampf deleted the ref/update-config-flag branch June 28, 2023 12:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@elchead elchead elchead approved these changes

@thomasten thomasten thomasten approved these changes

@derpsteb derpsteb Awaiting requested review from derpsteb

Assignees
No one assigned
Labels
breaking change Change breaks existing API or configuration.
Projects
None yet
Milestone
v2.9.0
Development

Successfully merging this pull request may close these issues.
4 participants
@miampf @derpsteb @elchead @thomasten
0