[CIRCLE-12217] Create namespace #28
Conversation
|
@hannahhenderson I will do a formal review soon, but I think we should make |
|
This is all UX, not go code review :) What about I also think we should wire it up so you can do:
rather than having to manually look up your org ID. This will require another roundtrip to query for the ID of the org called Super awesome to see this working! |
api/api.go
Outdated
| messages := []string{} | ||
|
|
||
| for i := range response.Errors { | ||
| messages = append(messages, response.Errors[i].Message) |
There was a problem hiding this comment.
Looks like you are not using the type field here. You should either use it, or drop it from the GQL request.
api/api.go
Outdated
|
|
||
| } | ||
|
|
||
| func createNamespaceQuery(ctx context.Context, logger *logger.Logger, name string, ownerId string, response interface{}, query string) error { |
There was a problem hiding this comment.
can you in-line this function into OrbCreateNamespace please? - I asked Eric to do the same at #27 (comment)
Indeed if we merge Eric's PR and you re-base on top you can re-use his new Errors struct to remove (response CreateNamespaceResponse) ToError()
hannahhenderson
force-pushed
the
create-namespace
branch
from
afeb333 to
97d51ac
Compare
|
@johnswanson I think that not forcing folks to use a Organization names aren't unique. Does it make more sense to add a separate "get organization id" CLI call (name + vcs type necessary), or to add all of that info to this call? |
api/api.go
Outdated
|
|
||
| err := graphQLclient.Run(ctx, request, &response) | ||
|
|
||
| fmt.Println(response) |
There was a problem hiding this comment.
I think you want to delete this line before merging? (In my editor, doing this would also get rid of the "fmt" import at the top on save. I think a default go tool does that, but not sure which.)
johnswanson
changed the title
johnswanson
force-pushed
the
create-namespace
branch
from
5bcce34 to
8140754
Compare
hannahhenderson
force-pushed
the
create-namespace
branch
2 times, most recently
from
a4910b0 to
a9d0184
Compare
Takes a positional 'name' argument and two flags: 'org-name' and (org)'vcs' Modify appendPostHandler to accept a struct of information
hannahhenderson
force-pushed
the
create-namespace
branch
from
a9d0184 to
4356630
Compare
hannahhenderson
changed the title
Codecov Report
@@ Coverage Diff @@
## master #28 +/- ##
==========================================
+ Coverage 40.82% 42.23% +1.41%
==========================================
Files 11 11
Lines 703 715 +12
==========================================
+ Hits 287 302 +15
+ Misses 393 390 -3
Partials 23 23
Continue to review full report at Codecov.
|
This PR adds "create namespace" to the list of orb commands (or, more specifically
createns, which is open to alternate naming suggestions).I used a lot of copy-pasting to make this work, thank you @eric-hu . I also benefited from some good old, "Please help me see the forest, oh :rubberduck: @johnswanson, because at this point in time I can only see the trees," unsticking.
Which is all to say, this PR works as expected, but I believe it is a good candidate for some DRY refactoring.
CIRCLE-12217
Examples of this in action in dev:
Ignore the weird UUID output in earlier responses, I deleted some print lines
Also, "GITHUB" and "BITBUCKET" can now be lower case
Help lists this information:
