Skip to content
This repository has been archived by the owner on Jan 11, 2025. It is now read-only.

fix: base_url formatting error #2

Merged
merged 1 commit into from
Aug 19, 2024
Merged

fix: base_url formatting error #2

merged 1 commit into from
Aug 19, 2024

Conversation

Kiwifuit
Copy link
Owner

@Kiwifuit Kiwifuit commented Aug 18, 2024

i seriously dont know why it manifested itself now out of all times

Summary by Sourcery

Fix the base_url formatting error in the MavenArtifactBuilder by removing the hardcoded 'https://' prefix, allowing the base URL to be correctly constructed.

Bug Fixes:

  • Fix the base_url formatting error by removing the hardcoded 'https://' prefix and ensuring the URL is correctly constructed from the provided base URL.

i seriously dont know why it manifested itself now out of all times
Copy link

sourcery-ai bot commented Aug 18, 2024

Reviewer's Guide by Sourcery

This pull request fixes a formatting error in the base_url construction within the MavenArtifactBuilder implementation. The change simplifies the base_url creation by removing an unnecessary format! macro call.

File-Level Changes

Files Changes
mar/src/types.rs Simplified base_url creation by removing the format! macro and https:// prefix

Tips
  • Trigger a new Sourcery review by commenting @sourcery-ai review on the pull request.
  • Continue your discussion with Sourcery by replying directly to review comments.
  • You can change your review settings at any time by accessing your dashboard:
    • Enable or disable the Sourcery-generated pull request summary or reviewer's guide;
    • Change the review language;
  • You can always contact us if you have any questions or feedback.

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey @Kiwifuit - I've reviewed your changes - here's some feedback:

Overall Comments:

  • Could you provide more context about why this change was necessary? The PR description doesn't explain the issue that's being addressed.
  • Consider adding a comment in the code explaining why the 'https://' prefix was removed, to prevent accidental reintroduction in the future.
  • Were any tests updated or added to cover this change? It would be helpful to ensure the new behavior is properly tested.
Here's what I looked at during the review
  • 🟢 General issues: all looks good
  • 🟢 Security: all looks good
  • 🟢 Testing: all looks good
  • 🟢 Complexity: all looks good
  • 🟢 Documentation: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.

@Kiwifuit Kiwifuit merged commit 8872240 into main Aug 19, 2024
1 check failed
@Kiwifuit Kiwifuit deleted the mar/repository branch August 19, 2024 10:55
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant