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

Add ticket events #180

Merged
merged 1 commit into from
Mar 16, 2024
Merged

Add ticket events #180

merged 1 commit into from
Mar 16, 2024

Conversation

sbrennan4
Copy link
Contributor

@sbrennan4 sbrennan4 commented Jan 16, 2024

Changes

Add ticket events.

Design doc: https://hackmd.io/9eTiXHu6RfedSuSwUMGTLw?both

Implements #164

Submitter Checklist

As the author of this PR, please check off the items in this checklist:

@sbrennan4 sbrennan4 force-pushed the ticket-events branch 4 times, most recently from 0e59c30 to ac8289c Compare January 18, 2024 18:21
@sbrennan4 sbrennan4 marked this pull request as ready for review January 18, 2024 18:24
@sbrennan4 sbrennan4 requested a review from a team as a code owner January 18, 2024 18:24
Copy link
Contributor

@afrittoli afrittoli left a comment

Choose a reason for hiding this comment

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

Thanks for this, it mostly looks good to me.

I left some comment, I think the schema might be a bit too specific in some places, and we could get along with a list of labels for many things like group, type, milestone and priority. It might be worth adding a few words to explain why we want include those fields as separate ones.

continuous-operations.md Show resolved Hide resolved
continuous-operations.md Outdated Show resolved Hide resolved
continuous-operations.md Outdated Show resolved Hide resolved
continuous-operations.md Outdated Show resolved Hide resolved
continuous-operations.md Outdated Show resolved Hide resolved
continuous-operations.md Show resolved Hide resolved
continuous-operations.md Show resolved Hide resolved
continuous-operations.md Outdated Show resolved Hide resolved
continuous-operations.md Outdated Show resolved Hide resolved
continuous-operations.md Show resolved Hide resolved
continuous-operations.md Outdated Show resolved Hide resolved
continuous-operations.md Outdated Show resolved Hide resolved
continuous-operations.md Outdated Show resolved Hide resolved
schemas/ticketcreated.json Outdated Show resolved Hide resolved
schemas/ticketupdated.json Show resolved Hide resolved
@afrittoli afrittoli added this to the v0.5 milestone Mar 4, 2024
@e-backmark-ericsson
Copy link
Contributor

According to the schemas in this PR, the subject.content object has a required 'uri' field, but the examples provided in the PR don't have that. @sbrennan4 , please validate the examples towards the corresponding json schemas.

@afrittoli
Copy link
Contributor

According to the schemas in this PR, the subject.content object has a required 'uri' field, but the examples provided in the PR don't have that. @sbrennan4 , please validate the examples towards the corresponding json schemas.

Thanks @e-backmark-ericsson - yeah, that's the reason for CI job failure

@afrittoli
Copy link
Contributor

@sbrennan4 you'll need to add a few words to the custom dictionary as well:

  • assignees
  • backend
  • cve

Copy link
Contributor

@e-backmark-ericsson e-backmark-ericsson left a comment

Choose a reason for hiding this comment

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

I left a minor request for updates, but I don't consider it important enough to hinder the approval. Such NITs could be cleaned up at a later stage.

continuous-operations.md Show resolved Hide resolved
continuous-operations.md Outdated Show resolved Hide resolved
@afrittoli afrittoli modified the milestones: v0.5, v0.4 Mar 12, 2024
Copy link
Contributor

@afrittoli afrittoli left a comment

Choose a reason for hiding this comment

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

Thanks, this looks good!
It would be nice if you included @e-backmark-ericsson suggestions but we can also do those as follow-up.

@afrittoli
Copy link
Contributor

@sbrennan4 could you please sign all the commits in the PR so this becomes mergeable?

Copy link
Contributor Author

@sbrennan4 sbrennan4 left a comment

Choose a reason for hiding this comment

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

I signed the commits following the directions from the CI check, which is satisfied with the string "Signed-off-by:". Seems the merge check wants a GPG or SSH key. Might want to update the CI check accordingly if possible.

schemas/ticketclosed.json Outdated Show resolved Hide resolved
continuous-operations.md Show resolved Hide resolved
@sbrennan4 sbrennan4 force-pushed the ticket-events branch 2 times, most recently from 5b07d1d to 85b03ac Compare March 14, 2024 18:34
Signed-off-by: Sean Brennan <[email protected]>
Copy link
Contributor

@afrittoli afrittoli left a comment

Choose a reason for hiding this comment

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

Thanks @sbrennan4 - great addition to CDEvents!

@afrittoli afrittoli merged commit 84cff29 into cdevents:main Mar 16, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants