Skip to content

Added missing attributes for webhook events #1240

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

Merged
merged 3 commits into from
Apr 22, 2025

Conversation

yarisvt
Copy link
Contributor

@yarisvt yarisvt commented Apr 10, 2025

Updated the following events with missing properties

  • AbstractPushEvent
  • BuildCommit
  • EventCommit
  • EventIssue
  • JobEvent
  • PipelineEvent
  • WikiPageEvent

Especially, PipelineEvent and JobEvent have changed.

PipelineEvent no longer uses a jobs array (Job), but a builds array (Build). I have not removed the jobs array for backwards compatibility.

JobEvent no longer uses job prefixed properties, but build prefixed properties. I have not removed the job prefixed properties for backwards compatibility. I did change the object_kind to build as it should be according to the docs (used to be job in version 12.x, but changed to build in version 13.x). This might break backwards compatibility.

@yarisvt yarisvt changed the title Added missing properties for webhook events Added missing attributes for webhook events Apr 10, 2025
/**
* Enum for the various Build status values.
*/
public enum BuildStatus {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I am wondering if this is not a duplication of the enum Constants.JobScope that we already have.

But BuildStatus seems a better name and feels more natural.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I actually copied JobStatus into BuildStatus, they're the same except for the name. But I figured it's more clear to distinguish between jobs and builds

Copy link
Collaborator

@jmini jmini left a comment

Choose a reason for hiding this comment

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

This looks a very good addition to the project. 👍

Copy link
Collaborator

@jmini jmini left a comment

Choose a reason for hiding this comment

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

Just there are build failure after your change.

I think this is due to the fact that there was already a BuildEvent with OBJECT_KIND build

case BuildEvent.OBJECT_KIND:
case IssueEvent.OBJECT_KIND:
case JobEvent.OBJECT_KIND:
case MergeRequestEvent.OBJECT_KIND:
case NoteEvent.OBJECT_KIND:
case PipelineEvent.OBJECT_KIND:
case PushEvent.OBJECT_KIND:
case TagPushEvent.OBJECT_KIND:
case WikiPageEvent.OBJECT_KIND:
case ReleaseEvent.OBJECT_KIND:
case DeploymentEvent.OBJECT_KIND:
case WorkItemEvent.OBJECT_KIND:

I didn't look into all the details, but different/more cleanup might be necessary.

@yarisvt
Copy link
Contributor Author

yarisvt commented Apr 15, 2025

You're right, there already is a BuildEvent, I missed that somehow. I've reverted the changes in JobEvent, and updated BuildEvent with some missing properties.

Copy link
Collaborator

@jmini jmini left a comment

Choose a reason for hiding this comment

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

No problem.

Thank you very much for the update and the 🧹 .

@jmini jmini merged commit b5b00e3 into gitlab4j:main Apr 22, 2025
2 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.

2 participants