DEPRECATION: add deprecation warning to this package #237
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-select component and updates the documentation generation process to reflect this change. It introduces a new documentation build script and a README template specifically for deprecated components. The location of the design tokens CSS file was also updated. 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! 😭 |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @DukeFerdinand - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider adding a script to automatically update the version number in the deprecation message when a new version of
@aurodesignsystem/auro-formkitis released. - It might be helpful to include the date of deprecation in the warning message.
Here's what I looked at during the review
- 🟡 General issues: 4 issues found
- 🟢 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.
| export const fileConfigs = () => [ | ||
| // README.md | ||
| { | ||
| identifier: "README.md", | ||
| input: "/Users/[email protected]/code/auro/auro-cli/.gitter-temp/multi-git-changer-2674357323/docTemplates/README.md", | ||
| output: "/Users/[email protected]/code/auro/auro-cli/.gitter-temp/multi-git-changer-2674357323/README.md", | ||
| }, | ||
| // index.md | ||
| { | ||
| identifier: "index.md", |
There was a problem hiding this comment.
suggestion: Avoid hard-coded absolute file paths in fileConfigs.
The absolute paths, which include specific usernames and directories, may cause portability issues. Consider making these paths relative or configurable to accommodate different environments.
Suggested implementation:
import path from 'path';
import { Logger } from "@aurodesignsystem/auro-library/scripts/utils/logger.mjs";const baseDir = process.cwd();
const tempDir = path.join(baseDir, '.gitter-temp', 'multi-git-changer-2674357323');
// Existing code...export const fileConfigs = () => [
// README.md
{
identifier: "README.md",
input: path.join(tempDir, 'docTemplates', 'README.md'),
output: path.join(tempDir, 'README.md'),
},
// index.md
{
identifier: "index.md",
input: path.join(tempDir, 'docs', 'partials', 'index.md'),If further file paths are specified later in the file, update them similarly to use path.join with the tempDir (or another configurable base) rather than absolute paths. Adjust the base directory configuration as needed for different environments.
| for (const fileConfig of fileConfigs(config)) { | ||
| try { |
There was a problem hiding this comment.
suggestion (bug_risk): fileConfigs is called with a parameter that it does not accept.
The function fileConfigs is defined with no parameters, but in processDocFiles it is invoked with 'config'. Either update fileConfigs to accept and use the parameter or remove the argument from the call.
Suggested implementation:
for (const fileConfig of fileConfigs()) {Make sure that the definition of fileConfigs does not expect a parameter. If it needs to use configuration information later, consider updating its definition accordingly.
| processDocFiles({ overwriteLocalCopies: false }) | ||
| .then(() => { | ||
| throw new Error("TESTING ERROR"); | ||
| Logger.log("Docs processed successfully"); | ||
| }) |
There was a problem hiding this comment.
suggestion (bug_risk): Unexpected error throw in the promise chain.
The .then() block immediately throws a 'TESTING ERROR', which seems unintentional for production code. This should be removed or properly conditioned.
| processDocFiles({ overwriteLocalCopies: false }) | |
| .then(() => { | |
| throw new Error("TESTING ERROR"); | |
| Logger.log("Docs processed successfully"); | |
| }) | |
| processDocFiles({ overwriteLocalCopies: false }) | |
| .then(() => { | |
| Logger.log("Docs processed successfully"); | |
| }) |
| | Description | Description of the component | `./docs/partials/description.md` | | ||
| | Use Cases | Examples for when to use this component | `./docs/partials/useCases.md` | | ||
| | Additional Information | For use to add any component specific information | `./docs/partials/readmeAddlInfo.md` | | ||
| | Component Example Code | HTML sample code of the components use | `./apiExamples/basic.html` | |
There was a problem hiding this comment.
issue (typo): Missing period at the end of the description.
| | Component Example Code | HTML sample code of the components use | `./apiExamples/basic.html` | | |
| | Component Example Code | HTML sample code of the components use. | `./apiExamples/basic.html` | |
| throw new Error("TESTING ERROR"); | ||
| Logger.log("Docs processed successfully"); |
There was a problem hiding this comment.
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.
DukeFerdinand
deleted the
deprecations/2025_03_05_add_deprecation_warnings
branch
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 is no longer supported and users are encouraged to migrate to the new Auro Formkit.
Enhancements:
Documentation: