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: Remove the Faraday multipart warning and tweak the error message raised at runtime when Faraday Multipart is not installed #1712

Merged
merged 1 commit into from
Sep 17, 2024

Conversation

Edouard-chin
Copy link
Contributor

Resolves #1701


Before the change?

  • A warning message related to the Faraday Multipart middleware is always printed at boot time.
    At runtime, a Faraday error is raised and the message of what happened is not clear.

After the change?

  • The warning message is not printed at boot time. A better error and message is raised at runtime.

Pull request checklist

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been reviewed and added / updated if needed (for bug fixes / features)

Does this introduce a breaking change?

Please see our docs on breaking changes to help!

  • Yes
  • No

- In 7bc6dd5, a warning message was
  added to let users know that faraday-multipart is required when
  using Faraday 2.0.

  This warning is not very helpful, and may lead to projects adding
  the gem to their dependencies even if they don't need it, as the
  faraday-multipart middleware is only used for uploading license for
  GitHub entrepise.

  This patch will avoid printing the warning unnecessarily but also
  display a better error message when ultimately
  the call to `conn.request :multipart` fails.

  Fix octokit#1701
Copy link

github-actions bot commented Sep 3, 2024

👋 Hi! Thank you for this contribution! Just to let you know, our GitHub SDK team does a round of issue and PR reviews twice a week, every Monday and Friday! We have a process in place for prioritizing and responding to your input. Because you are a part of this community please feel free to comment, add to, or pick up any issues/PRs that are labeled with Status: Up for grabs. You & others like you are the reason all of this works! So thank you & happy coding! 🚀

@Edouard-chin Edouard-chin changed the title Raise an error at runtime when Faraday Multipart is not installed Remove the Faraday multipart warning and tweak the error message raised at runtime when Faraday Multipart is not installed Sep 3, 2024
Copy link
Contributor

@nickfloyd nickfloyd left a comment

Choose a reason for hiding this comment

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

Thank you for making this better! ❤️ We'll need to ensure with future SDKs that might require a form requests from a kv hash make sure to include this check as well.

@nickfloyd nickfloyd changed the title Remove the Faraday multipart warning and tweak the error message raised at runtime when Faraday Multipart is not installed fix: Remove the Faraday multipart warning and tweak the error message raised at runtime when Faraday Multipart is not installed Sep 17, 2024
@nickfloyd nickfloyd added the Type: Bug Something isn't working as documented label Sep 17, 2024
@nickfloyd
Copy link
Contributor

Labeling as bug because of the poor user experience and noise that is getting logged.

@nickfloyd nickfloyd merged commit 6c62c46 into octokit:main Sep 17, 2024
16 checks passed
@Edouard-chin Edouard-chin deleted the ec-faraday-runtime branch October 4, 2024 10:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Something isn't working as documented
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

[BUG]: faraday-multipart warning always printed
2 participants