Skip to content
This repository has been archived by the owner on Mar 11, 2021. It is now read-only.

Add help text in README for creation, retrieval, listing of WorkItemTypes #195

Merged
merged 1 commit into from Sep 20, 2016

Conversation

pranavgore09
Copy link
Contributor

@pranavgore09 pranavgore09 commented Sep 6, 2016

taken from examples/workitemtype.adoc

Fixes #208


This change is Reviewable

@kwk
Copy link
Collaborator

kwk commented Sep 6, 2016

Hi @pranavgore09 . In principle I'd say this is a good thing to have in the README but we might want to inform the user about potential problems simply because of duplicate keys. The user can only create the Epic work item type once and after that she or he will get the duplicate key error.

@pranavgore09
Copy link
Contributor Author

yes @kwk you are right, updating....

@aslakknutsen
Copy link
Contributor

Wondering if we should move to testable examples in the source code instead of writing everything up in the README. The README is getting quite large with light and deep setup info and other things. It looks like the project is super complicated, when really it's not.

We could in the README just point to the examples folder, and in the example folder have testable examples (testable in the sense of being automatically tested and verified): https://blog.golang.org/examples

?

@pranavgore09
Copy link
Contributor Author

pranavgore09 commented Sep 6, 2016

goa-design supports example attribute for fields which will reflect in swagger. - doc

@kwk
Copy link
Collaborator

kwk commented Sep 6, 2016

@aslakknutsen @pranavgore09 or we could use those examples for tests with the final deploy image.

@aslakknutsen
Copy link
Contributor

aslakknutsen commented Sep 6, 2016

@kwk That's what I'm wondering about... related #178


----
$ ./bin/alm-cli create workitem --payload '{"type": "1" , "name": "some name", "fields": { "system.owner": "tmaeder", "system.state": "open" }}' -H localhost:8080
$ ./bin/alm-cli create workitem --payload '{"type": "Epic" , "name": "some name", "fields": { "system.owner": "tmaeder", "system.state": "open" }}' -H localhost:8080
Copy link
Contributor

Choose a reason for hiding this comment

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

Epic? Epic is not available. And It looks like "system.state": "open" is not available either. Should be "new" instead. And "system.title" is missing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@alexeykazakov : yes you are right few keys are missing and have different values. But this is just an example data for creating WIT. With help of this dummy data, user can build n number of WITs with different possible values.

And I think somewhere else in the document we should teach the user about available (bootstrapped) WITs.

Copy link
Contributor

Choose a reason for hiding this comment

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

Remove "name", it's no longer in API

Copy link
Contributor

Choose a reason for hiding this comment

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

@alexeykazakov @pranavgore09 The WorkItem definition match the WorkitemType created further up, so not wrong in that sense. e.g. system.state is just a string.

@pranavgore09 If you add some text like: Based on the WorkItemType we created above, we can now create a WorkItem...

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, ok. Makes sense now. Thanks for clarification.

@pranavgore09
Copy link
Contributor Author

@kwk : Yes but before that I want to understand - when we do make test-all, these test examples will run or not ?
And will these examples make use of API call OR it will call create method of workitem_repository ? And for final deploy image, which database will be used for these examples ? because one can run examples 100 times, should there be 100 WITs every time ? (considering unique key constraints and all)

Create a work item.
Create a work item type.
----
./bin/alm-cli create workitemtype --payload '{"extendedTypeName":null,"fields":{"system.owner":{"Type":{"Kind":"user"},"Required":true},"system.state":{"Type":{"Kind":"string"},"Required":false}},"name":"Epic"}' -H localhost:8080 --pp
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove extendedTypeName

@pranavgore09
Copy link
Contributor Author

Review status: 0 of 1 files reviewed at latest revision, 2 unresolved discussions, some commit checks failed.


README.adoc, line 265 [r1] (raw file):

Previously, aslakknutsen (Aslak Knutsen) wrote…

Remove extendedTypeName

Because it is optional ? and we are not focusing "extend" yet ?

README.adoc, line 283 [r1] (raw file):

Previously, alexeykazakov (Alexey Kazakov) wrote…

Ah, ok. Makes sense now. Thanks for clarification.

Yup, I thought "We need to use name of work item type in the `type` field below." will be helpful for reader, but yeah will make it more verbose.

Comments from Reviewable

@aslakknutsen
Copy link
Contributor

README.adoc, line 265 [r1] (raw file):

Previously, pranavgore09 (Pranav Gore) wrote…

Because it is optional ? and we are not focusing "extend" yet ?

Yes, and it doesn't add anything to the example.

Comments from Reviewable

@pranavgore09
Copy link
Contributor Author

Review status: 0 of 1 files reviewed at latest revision, 2 unresolved discussions, some commit checks failed.


README.adoc, line 265 at r1 (raw file):

Previously, aslakknutsen (Aslak Knutsen) wrote…

Yes, and it doesn't add anything to the example.

Yup removed.

Comments from Reviewable

@codecov-io
Copy link

codecov-io commented Sep 19, 2016

Current coverage is 22.08% (diff: 100%)

Merging #195 into master will not change coverage

@@             master       #195   diff @@
==========================================
  Files            32         32          
  Lines          1173       1173          
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
  Hits            259        259          
  Misses          863        863          
  Partials         51         51          

Powered by Codecov. Last update fc426e5...ee0f7cc

@pranavgore09
Copy link
Contributor Author

pranavgore09 commented Sep 20, 2016

@aslakknutsen : this is a small doc change. I think it is ready to merge. Can you please take a look ?
Fixes #208

@aslakknutsen
Copy link
Contributor

@pranavgore09 Can you rebase and re-push

@aslakknutsen
Copy link
Contributor

@pranavgore09
Copy link
Contributor Author

Rebase done.
Thanks @aslakknutsen . Yes this does not fix #187 . added by mistake.

@aslakknutsen aslakknutsen merged commit 845254f into fabric8-services:master Sep 20, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants