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

add ignore reviewers change to alwaysIgnoreNonCodeRelatedUpdates #295

Closed
wants to merge 0 commits into from
Closed

add ignore reviewers change to alwaysIgnoreNonCodeRelatedUpdates #295

wants to merge 0 commits into from

Conversation

mifitous
Copy link
Contributor

@mifitous mifitous commented Apr 4, 2023

In case that reviewers list changed, but flag alwaysIgnoreNonCodeRelatedUpdates was checked, ignore build

  • Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
  • Ensure that the pull request title represents the desired changelog entry
  • Please describe what you did
  • Link to relevant issues in GitHub or Jira
  • Link to relevant pull requests, esp. upstream and downstream changes
  • Ensure you have provided tests - that demonstrates feature works or fixes the issue

@mifitous mifitous requested a review from jetersen as a code owner April 4, 2023 13:26
@jetersen jetersen added the enhancement New feature or request label Apr 4, 2023
@jetersen
Copy link
Member

jetersen commented Apr 4, 2023

@mifitous https://ci.jenkins.io/blue/organizations/jenkins/Plugins%2Fgitlab-branch-source-plugin/detail/PR-295/1/pipeline/43#step-66-log-120

getReviewers do not exist.

@mifitous
Copy link
Contributor Author

mifitous commented Apr 4, 2023

@mifitous https://ci.jenkins.io/blue/organizations/jenkins/Plugins%2Fgitlab-branch-source-plugin/detail/PR-295/1/pipeline/43#step-66-log-120

getReviewers do not exist.

Hi @jetersen
It was merged in gitlab4j/gitlab4j-api#916 but I do not understand why it is still not presented in our code. Any idea?

@jetersen
Copy link
Member

jetersen commented Apr 4, 2023

@mifitous

gitlab-branch-source-plugin/pom.xml

Lines 36 to 39 in ad1ce6d

<dependency>
<groupId>io.jenkins.plugins</groupId>
<artifactId>gitlab-api</artifactId>
</dependency>

https://github.com/jenkinsci/bom/blob/bd41cba72c8231b42ad4fc355a42d92e14fad96f/bom-weekly/pom.xml#L139-L141

https://github.com/jenkinsci/gitlab-api-plugin/blob/3b564bafb14281a9e0e85367c4473eafedfb6f82/pom.xml#L77-L79

https://github.com/jenkinsci/gitlab-api-plugin/blob/3b564bafb14281a9e0e85367c4473eafedfb6f82/pom.xml#L17

@mifitous
Copy link
Contributor Author

mifitous commented Apr 4, 2023

@mifitous

gitlab-branch-source-plugin/pom.xml

Lines 36 to 39 in ad1ce6d

<dependency>
<groupId>io.jenkins.plugins</groupId>
<artifactId>gitlab-api</artifactId>
</dependency>

https://github.com/jenkinsci/bom/blob/bd41cba72c8231b42ad4fc355a42d92e14fad96f/bom-weekly/pom.xml#L139-L141

https://github.com/jenkinsci/gitlab-api-plugin/blob/3b564bafb14281a9e0e85367c4473eafedfb6f82/pom.xml#L77-L79

https://github.com/jenkinsci/gitlab-api-plugin/blob/3b564bafb14281a9e0e85367c4473eafedfb6f82/pom.xml#L17

so I'll wait for it...
Thanks

@mifitous mifitous marked this pull request as draft April 4, 2023 14:15
@jetersen
Copy link
Member

jetersen commented Apr 4, 2023

Feel free to submit a PR to update gitlab4j in the gitlab-api-plugin. I don't think Dependabot have caught it yet.

@jetersen
Copy link
Member

jetersen commented Apr 4, 2023

jenkinsci/gitlab-api-plugin#41 FYI

@mifitous
Copy link
Contributor Author

mifitous commented Apr 4, 2023

Thanks

@jetersen
Copy link
Member

jetersen commented Apr 7, 2023 •
edited
Loading

@mifitous You can rebase the PR on default branch now to get the updated GitLab API :)
#298

@mifitous mifitous marked this pull request as ready for review April 7, 2023 14:10
@mifitous
Copy link
Contributor Author

mifitous commented Apr 7, 2023 •
edited
Loading

@mifitous You can rebase the PR on default branch now to get the updated GitLab API :) #298

Hi @jetersen
Merged, but still not passing. Seems gitlab4j/gitlab4j-api#916 was merged but the gitlab4j-api was not released with this MR.
gitlab4j/gitlab4j-api@gitlab4j-api-5.0.1...gitlab4j-api-5.1.0

@jetersen
Copy link
Member

jetersen commented Apr 7, 2023

@mifitous I got to the same conclusion 😓

Let's hope we can get a release with the changes :)

@jmini
Copy link
Contributor

jmini commented Apr 13, 2023

The change is available in 5.2.0:

I think you need jenkinsci/gitlab-api-plugin#44

@mifitous mifitous force-pushed the add-ignore-reviewers-to-alwaysIgnoreNonCodeRelatedUpdates branch 3 times, most recently from 1a0435f to c4bfe22 Compare April 18, 2023 08:57
@jmini
Copy link
Contributor

jmini commented May 30, 2023

I think the approach of checking the changes value for the alwaysIgnoreNonCodeRelatedUpdates ("Ignore builds for non-code related MR updates") feature is not good enough.

As I wrote in #323 the presence or the absence of object_attributes.oldrev seems to be the correct approach to cover all cases.

@mifitous
Copy link
Contributor Author

@mifitous I got to the same conclusion 😓

Let's hope we can get a release with the changes :)

Hi @jetersen : indeed, now it compile fine with the new version of the pom.xml. all tests passed

jetersen
jetersen reviewed Jun 19, 2023
View reviewed changes
pom.xml Outdated
<artifactId>bom-2.361.x</artifactId>
<version>2025.v816d28f1e04f</version>
<artifactId>bom-2.387.x</artifactId>
<version>2179.v0884e842b_859</version>
Copy link
Member

Choose a reason for hiding this comment

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

Can we do this in a separate PR?

Or is it asking too much ? 😓

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @jetersen
sure: #334

@jetersen
Copy link
Member

@mifitous what do you think @jmini comment above

I think the approach of checking the changes value for the alwaysIgnoreNonCodeRelatedUpdates ("Ignore builds for non-code related MR updates") feature is not good enough.

As I wrote in #323 the presence or the absence of object_attributes.oldrev seems to be the correct approach to cover all cases.

@mifitous
Copy link
Contributor Author

@mifitous what do you think @jmini comment above

I think the approach of checking the changes value for the alwaysIgnoreNonCodeRelatedUpdates ("Ignore builds for non-code related MR updates") feature is not good enough.
As I wrote in #323 the presence or the absence of object_attributes.oldrev seems to be the correct approach to cover all cases.

Hi @jetersen and @jmini

as gitlab4j/gitlab4j-api#980 is now merged, I'll go for implementation of it
I'll correct this PR

@jetersen
Copy link
Member

@mifitous Much appreciated and sorry for the rework 😅

@mifitous mifitous force-pushed the add-ignore-reviewers-to-alwaysIgnoreNonCodeRelatedUpdates branch from 24b3d98 to 9df28c7 Compare June 19, 2023 11:28
@mifitous mifitous closed this Jun 19, 2023
@mifitous mifitous force-pushed the add-ignore-reviewers-to-alwaysIgnoreNonCodeRelatedUpdates branch from 9df28c7 to f7d1844 Compare June 19, 2023 11:34
@mifitous
Copy link
Contributor Author

Created new PR at: #335

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jetersen jetersen jetersen approved these changes

Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mifitous @jetersen @jmini