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

Implement Logging for Bundler v1 Deprecation Errors and Warnings #10333

Closed
wants to merge 7 commits into from

Conversation

kbukum1
Copy link
Contributor

@kbukum1 kbukum1 commented Jul 31, 2024

Title: Implement Logging for Bundler v1 Deprecation Errors and Warnings

What are you trying to accomplish?

This PR introduces logging for deprecation warnings and errors specifically for Bundler v1. To achieve this, a generic framework is created in the UpdateHelpers module that can be extended for other ecosystems in the future.

Why:

  • To notify users of the deprecation of Bundler v1 and ensure they are aware that it is no longer supported.
  • To create a flexible and reusable logging system that can be applied to other package managers as needed.

This change ensures users are informed when using deprecated versions of Bundler and provides a structured approach for handling similar deprecations in other ecosystems.

Anything you want to highlight for special attention from reviewers?

The implementation includes:

  • Adding methods to the UpdateHelpers module to log warnings and errors for deprecated versions.
  • Applying these methods specifically to Bundler v1 in this PR.

This approach allows for easy extension to other package managers in the future, ensuring consistency and maintainability.

How will you know you've accomplished your goal?

  • Successfully logging deprecation warnings and errors for Bundler v1.
  • Ensuring that the job fails with an appropriate error message when using Bundler v1.
  • Verification through tests that the correct messages and error logs are recorded when Bundler v1 is encountered.

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.

supported_versions: T::Array[String]
).void
end
def record_deprecation_warning_for_eco_system(
Copy link
Member

Choose a reason for hiding this comment

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

wondering if we could pass the mode as an enum? E.g. as WARN or ERROR; that way we can reuse the same function.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will check that and I think it is possible and depends to how it is used it may be better solution.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The code is moved #10466

"Supported versions are: #{supported_versions.join(', ')}. Future updates are not guaranteed."
)

service.record_update_job_error(
Copy link
Member

Choose a reason for hiding this comment

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

Wondering if we should refactor this by renaming the function to more accurately capture its intent? This can be a follow up PR though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you mean changing the service one or here. Actually in service we have this method which is sending the given params to API UI.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The code is moved #10466

@kbukum1 kbukum1 force-pushed the kamil/bundler_v1_deprecation_error branch from 44fa7e2 to 7cb269f Compare August 1, 2024 18:48
@kbukum1 kbukum1 self-assigned this Aug 2, 2024
@kbukum1 kbukum1 force-pushed the kamil/bundler_v1_deprecation_error branch from 2043b35 to 2c28cb7 Compare August 2, 2024 20:38
@kbukum1 kbukum1 force-pushed the kamil/bundler_v1_deprecation_error branch from d73843f to d606d17 Compare August 16, 2024 17:43
@kbukum1 kbukum1 force-pushed the kamil/bundler_v1_deprecation_error branch from d606d17 to dac2832 Compare August 19, 2024 21:01
@kbukum1
Copy link
Contributor Author

kbukum1 commented Aug 19, 2024

The branch name is changed for implementing only deprecation warnings for bundler v1.
New PR: #10466

@kbukum1 kbukum1 closed this Aug 19, 2024
@kbukum1 kbukum1 deleted the kamil/bundler_v1_deprecation_error branch August 22, 2024 20:23
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