-
Notifications
You must be signed in to change notification settings - Fork 926
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
Show ignore conditions in a request body #7654
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, nice work!
409247f
to
ca4115b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How to I stop what I'm messing up?
|
||
# Filter out the conditions where from_config_file is false and dependency is in @dependencies | ||
valid_ignore_conditions = @ignore_conditions.select do |ic| | ||
!ic[:from_config_file] && dependencies.any? { |dep| dep.name == ic[:dependency_name] } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@honeyankit where does this from_config_file
value come from? I don't see it in https://github.com/github/dependabot-api/blob/9fc5f8ad58c303bb9a1df9e460ee812d5350e797/app/models/update_job.rb#L184-L187
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jurre: We also need to modify this function. My earlier assumption was wrong about the attributes of ignore_conditions, We have to replace from_config_file
with source
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The source will contain string with info whether it was invoked via config file or via @dependabot ignore command
. It will have to match the ignore command here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have implemented @dependbot show <dep -name> ignore conditions
which is something similar to this. You can get context from that and modify the ignore_conditions_table
method.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have added the field created_at
and updated_at
in update job on this PR: https://github.com/github/dependabot-api/pull/4319
* Added ignore condition to message builder class to show the ignore conditions of the dependencies in the PR body.
Context
To remove ignore conditions on the dependencies on a given pull request user needs to have option to view those conditions. This task is to implement that for the user. The PR body should look something like below:
PR Body
Dependabot Ignore Conditions Applied on the Pull Request
The user will use the ignore condition mentioned in the PR body and will use the upcoming new unignore command to remove those conditions on a specific dependency:
Approach
In the PR body, displaying top 20 lasted ignore conditions (if any) sorted by
created_at
orupdated_at
for dependencies in the pull request.The reason for the limiting to show only 20 ignore conditions on the PR is body is because at this moment "GitHub limits PR/comment descriptions to a max of 65,536 characters".
Fix: #2255