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 missing site_url bug #37

Merged
merged 5 commits into from
Jan 24, 2024
Merged

Fix missing site_url bug #37

merged 5 commits into from
Jan 24, 2024

Conversation

glenn-jocher
Copy link
Member

No description provided.

Copy link
Contributor

@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.

PR Type: Bug fix

PR Summary: The pull request addresses an issue where the 'site_url' configuration was not being checked before use, which could lead to incorrect behavior or errors. The changes include a warning message when 'site_url' is not set and a fallback to an empty string to prevent concatenation issues.

Decision: Comment

📝 Type: 'Bug fix' - not supported yet.
  • Sourcery currently only approves 'Typo fix' PRs.
✅ Issue addressed: this change correctly addresses the issue or implements the desired feature.
No details provided.
📝 Complexity: the changes are too large or complex for Sourcery to approve.
  • Unsupported files: the diff contains files that Sourcery does not currently support during reviews.

General suggestions:

  • Consider using a logging mechanism instead of direct print statements for warnings to allow better control in different environments.
  • Review the URL concatenation logic to ensure that it correctly handles edge cases, such as missing or malformed 'site_url' values.
  • Implement a more explicit conditional expression for handling the absence of 'site_url' to provide clearer error handling or fallback strategies.
  • Ensure that there is clear handling or logging when features are disabled through configuration flags, to avoid confusion about the plugin's behavior.

Thanks for using Sourcery. We offer it for free for open source projects and would be very grateful if you could help us grow. If you like it, would you consider sharing Sourcery on your favourite social media? ✨

Share Sourcery

Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.

plugin/main.py Outdated Show resolved Hide resolved
plugin/main.py Show resolved Hide resolved
plugin/main.py Show resolved Hide resolved
@glenn-jocher glenn-jocher merged commit eb4aa60 into main Jan 24, 2024
1 check passed
@glenn-jocher glenn-jocher deleted the fix branch January 24, 2024 13:16
@glenn-jocher glenn-jocher linked an issue Jan 24, 2024 that may be closed by this pull request
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.

TypeError: unsupported operand type(s) for +: 'NoneType' and 'str'
2 participants