-
Notifications
You must be signed in to change notification settings - Fork 1
DEPRECATION: add deprecation warning to this package #161
DEPRECATION: add deprecation warning to this package #161
Conversation
As part of the new formkit release, all old form element repositories are being deprecated.
Reviewer's Guide by SourceryThis pull request adds a deprecation warning to the auro-checkbox component and updates the documentation generation process to handle deprecated components. A new documentation template was created to include the deprecation warning in the generated README file. The build process was updated to use a custom script that uses the new template. No diagrams generated as the changes look simple and do not need a visual representation. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Surge demo deployment failed! 😭 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @DukeFerdinand - I've reviewed your changes and found some issues that need to be addressed.
Blocking issues:
- Remove the testing error throw from production code. (link)
Overall Comments:
- Consider adding a step to remove the deprecated packages from any monorepo tooling.
- Be sure to update the main auro.alaskaair.com site to reflect the deprecation.
Here's what I looked at during the review
- 🔴 General issues: 1 blocking issue, 2 other issues
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
||
processDocFiles({ overwriteLocalCopies: false }) | ||
.then(() => { | ||
throw new Error("TESTING ERROR"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
issue (bug_risk): Remove the testing error throw from production code.
Throwing a test error immediately after successful processing forces an error state in production. This appears to be a leftover from debugging and should be removed or conditionally enabled.
// setup | ||
await templateFiller.extractNames(); | ||
|
||
for (const fileConfig of fileConfigs(config)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion: Clarify unused 'config' parameter in fileConfigs call.
The fileConfigs function is defined without parameters yet is being called with 'config'. Either update fileConfigs to accept and use the config, or remove the unused parameter from the call to prevent confusion for future maintainers.
for (const fileConfig of fileConfigs(config)) { | |
for (const fileConfig of fileConfigs()) { |
|
||
> **WARNING:** This component is deprecated and is no longer supported. Please migrate to the new [Auro Formkit](https://www.npmjs.com/package/@aurodesignsystem/auro-formkit) instead. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion: Consider adding the deprecation notice to the README template instead of the individual README file.
This would ensure that the deprecation notice is included automatically when the README is generated.
throw new Error("TESTING ERROR"); | ||
Logger.log("Docs processed successfully"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion (code-quality): Remove unreachable code. (remove-unreachable-code
)
throw new Error("TESTING ERROR"); | |
Logger.log("Docs processed successfully"); | |
throw new Error("TESTING ERROR"); | |
Explanation
Statements after areturn
, break
, continue
or throw
will never be executed.Leaving them in the code confuses the reader, who may believe that these
statements have some effect. They should therefore be removed.
Resolves AlaskaAirlines/auro-formkit#394, adding deprecation warning to old form element repositories.
Summary by Sourcery
Adds a deprecation warning to the component and updates the documentation to reflect this change. The component now displays a warning message indicating that it is deprecated and users should migrate to the new Auro Formkit.
Documentation: