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

feat: make organization ID generally available [HEAD-855] #4887

Merged
merged 3 commits into from Oct 10, 2023

Conversation

PeterSchafer
Copy link
Contributor

@PeterSchafer PeterSchafer commented Oct 2, 2023

What does this PR do?

In the Legacy CLI, introduce an additional config value that will always hold the orgId provided by the Extensible CLI.

Where should the reviewer start?

  • cliv2/pkg/basic_workflows/legacycli.go
  • src/lib/config/index.ts

How should this be manually tested?

Any background context you want to provide?

What are the relevant tickets?

Screenshots

Additional questions

@github-actions
Copy link
Contributor

github-actions bot commented Oct 5, 2023

Warnings
⚠️

You've modified files in src/ directory, but haven't updated anything in test folder. Is there something that could be tested?

Generated by 🚫 dangerJS against c510302

@PeterSchafer PeterSchafer changed the title feat: make organization ID generally available feat: make organization ID generally available [HEAD-855] Oct 9, 2023
@PeterSchafer PeterSchafer marked this pull request as ready for review October 9, 2023 16:11
@PeterSchafer PeterSchafer requested review from a team as code owners October 9, 2023 16:11
var p WrapperProxy
p.DebugLogger = debugLogger
p.cliVersion = cliVersion
p.addHeaderFunc = func(request *http.Request) error { return nil }

cacheDirectory := config.GetString(configuration.CACHE_PATH)
Copy link
Contributor

Choose a reason for hiding this comment

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

praise: nice to include this here

func NewCLIv2(cacheDirectory string, debugLogger *log.Logger) (*CLI, error) {
func NewCLIv2(config configuration.Configuration, debugLogger *log.Logger) (*CLI, error) {

cacheDirectory := config.GetString(configuration.CACHE_PATH)
Copy link
Contributor

Choose a reason for hiding this comment

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

question: should we ensure that the directory is writeable?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea, I think this should be done more centrally. In general the default cache path is checked for being writeable.

@PeterSchafer PeterSchafer enabled auto-merge (squash) October 10, 2023 13:31
@PeterSchafer PeterSchafer merged commit b51ed86 into master Oct 10, 2023
9 checks passed
@PeterSchafer PeterSchafer deleted the feat/HEAD-855_supply_org_id branch October 10, 2023 13:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants