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.

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

feat(task): Add task types #14567

Merged
merged 1 commit into from
Aug 6, 2019
Merged

feat(task): Add task types #14567

merged 1 commit into from
Aug 6, 2019

Conversation

lyondhill
Copy link
Contributor

@lyondhill lyondhill commented Aug 5, 2019 •
edited
Loading

Add a type to task's to allow tasks to be filterable.

@lyondhill lyondhill requested review from a team as code owners August 5, 2019 21:47
@lyondhill lyondhill requested a review from a team August 5, 2019 21:49
@ghost ghost requested review from GeorgeMac and removed request for a team August 5, 2019 21:49
gshif
gshif reviewed Aug 5, 2019
View reviewed changes
Copy link
Contributor

@gshif gshif left a comment

Choose a reason for hiding this comment

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

@lyondhill , can you add more description for task types in the swagger: what is task type, valid/invalid task types,etc.

chnn
chnn approved these changes Aug 5, 2019
View reviewed changes
http/swagger.yml Show resolved Hide resolved
@lyondhill
Copy link
Contributor Author

@gshif We are currently not limiting task types (other than using a "wildcard" as the task type on create). I can make the description in the swagger more descriptive.

@gshif
Copy link
Contributor

gshif commented Aug 5, 2019

That would be great

@lyondhill lyondhill changed the title feat(task): add task types feat(task): Aadd task types Aug 5, 2019
@lyondhill lyondhill changed the title feat(task): Aadd task types feat(task): Add task types Aug 5, 2019
GeorgeMac
GeorgeMac reviewed Aug 6, 2019
View reviewed changes
kv/task.go Outdated Show resolved Hide resolved
GeorgeMac
GeorgeMac approved these changes Aug 6, 2019
View reviewed changes
Copy link
Contributor

@GeorgeMac GeorgeMac left a comment

Choose a reason for hiding this comment

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

I added a couple take it or leave it comments. Otherwise this LGTM.

@lyondhill lyondhill merged commit e922c8a into master Aug 6, 2019
@lyondhill lyondhill deleted the task/type branch August 6, 2019 16:27
@noramullen1 noramullen1 mentioned this pull request Aug 8, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gshif gshif gshif left review comments

@ebb-tide ebb-tide ebb-tide approved these changes

@kelwang kelwang kelwang approved these changes

@GeorgeMac GeorgeMac GeorgeMac approved these changes

@imogenkinsman imogenkinsman imogenkinsman approved these changes

@chnn chnn chnn approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@lyondhill @gshif @chnn @imogenkinsman @GeorgeMac @kelwang @ebb-tide