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

fix: check for a changelog text before returning a changelog url #10522

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

zeroxdeadx
Copy link

@zeroxdeadx zeroxdeadx commented Aug 29, 2024

What are you trying to accomplish?

I'm trying to fix a problem where dependabot creates a pull request, and its commit message contains a url to a file that is not a changelog. The pull request message, in its turn, doesn't contain that url.

image
image

Anything you want to highlight for special attention from reviewers?

Perhaps we could solve this issue by allowing a changelog file to have only the '.md' extension or no extension at all (like "changelog", "changelog.md", "history", "history.md" and so on). I haven't chosen this approach because (maybe) some projects use files with different extensions for their release notes.

How will you know you've accomplished your goal?

I've written a test to ensure the issue is fixed.

Checklist

  • I have run the complete test suite to ensure all tests and linters pass.
  • I have thoroughly tested my code changes to ensure they work as expected, including adding additional tests for new functionality.
  • I have written clear and descriptive commit messages.
  • I have provided a detailed description of the changes in the pull request, including the problem it addresses, how it fixes the problem, and any relevant details about the implementation.
  • I have ensured that the code is well-documented and easy to understand.

@zeroxdeadx zeroxdeadx requested a review from a team as a code owner August 29, 2024 22:51
@zeroxdeadx zeroxdeadx force-pushed the fix/wrong-changelog branch 2 times, most recently from 0a35f59 to 2af7e98 Compare August 30, 2024 16:26
@zeroxdeadx zeroxdeadx marked this pull request as draft August 30, 2024 16:31
@zeroxdeadx zeroxdeadx marked this pull request as ready for review August 30, 2024 16:34
@zeroxdeadx zeroxdeadx force-pushed the fix/wrong-changelog branch 3 times, most recently from 997359d to 5e0e582 Compare September 4, 2024 20:36
Currently, the changelog_url function (message_builder.rb) returns a
url to the first changelog file with a name from the list defined in
Dependabot::MetadataFinders::Base::ChangelogFinder::CHANGELOG_NAMES.

This causes it to sometimes return a url to a file, which is definitely
not a changelog (e.g. some script used to make a release of a library).
The commit message ends up containing a url that shouldn't be there.

The solution is to check the contents of the changelog file by invoking
the changelog_text function (metadata_finders/base.rb) in an 'if'
statement before returning the result of the changelog_url function.

The same approach is used in the changelog_cascade function, which is
used for building a pull request message (metadata_presenter.rb).

Signed-off-by: zeroxdead <zeroxdead@proton.me>
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