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: handle invalid directory errors when creating atomic component #1327

Closed
wants to merge 14 commits into from

Conversation

dmbrooke
Copy link

@dmbrooke dmbrooke commented Sep 22, 2023

KIT-2722

Unfortunately adding to the pile that is CDX-1428, but I couldn't find any simple/quick way to share the error class and behaviour between packages without doing larger changes.

Proposed changes

It would be useful to provide a helpful error message when attempting to create an atomic component in an invalid directory. Thus, I added the necessary to the create-atomic-component and create-atomic-result-component packages.

Testing

UNIT TESTS ARE STILL IN PROGRESS. For now, you can test the changes by following these steps:

  • Build this branch (npm run build)
  • Open a non-node project directory in your terminal (e.g., a java project)
  • Try to create a component in said directory using your local build.
    • If the directory is adjacent to where you have cloned the CLI project, your command will look something like
    ../cli/packages/cli/core/bin/dev atomic:cmp --type=result test-result-component
    
  • Confirm that a clear error message outputs in your terminal, explaining that the current directory is invalid.

@dmbrooke dmbrooke requested a review from a team as a code owner September 22, 2023 19:01
@dmbrooke dmbrooke requested review from olamothe, y-lakhdar and mrrajamanickam-coveo and removed request for a team September 22, 2023 19:01
@github-actions
Copy link
Contributor

github-actions bot commented Sep 22, 2023

Dependency Review

✅ No vulnerabilities or license issues found.

Snapshot Warnings

⚠️: No snapshots were found for the head SHA 2ba4317.
Ensure that dependencies are being submitted on PR branches and consider enabling retry-on-snapshot-warnings. See the documentation for more information and troubleshooting advice.

Scanned Manifest Files

@github-actions
Copy link
Contributor

Thanks for your contribution @dmbrooke !
When your pull-request is ready to be merged, check the box below to merge it

  • Merge! :shipit:

@github-actions
Copy link
Contributor

Pull Request Report

PR Title

✅ Title follows the conventional commit spec.

@dmbrooke dmbrooke marked this pull request as draft September 22, 2023 19:33
@dmbrooke dmbrooke marked this pull request as ready for review September 28, 2023 14:20
if (cwdFiles.length > 0 && !hasPackageInCwd) {
handleErrors(
new InvalidProjectDirectory(
'Current working directory is either not empty or not an npm project (no package.json found). Please try again in an empty directory.'
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
'Current working directory is either not empty or not an npm project (no package.json found). Please try again in an empty directory.'
'Current working directory is either not empty or not an npm project (no package.json found). Please try again in a valid project'

maybe specifying how to initialize an atomic project

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe link to some relevant doc which explains the process e.g https://docs.coveo.com/en/cli/ or something more specific if available

Copy link
Collaborator

Choose a reason for hiding this comment

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

Disagreeing here: The create-atomic-(result)-component packages are initializers that are agnostic of the concept of atomic project.
i.e. you can use them without an atomic project.

Copy link
Author

Choose a reason for hiding this comment

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

I personally think it's valid to refer to it as a valid project as that isn't specific to an atomic project. I think I'll update the link for this description to instead point to the atomic:cmp command documentation as it clearly outlines in what contexts the command will work:
https://docs.coveo.com/en/cli/#coveo-atomiccmp-name

if (cwdFiles.length > 0 && !hasPackageInCwd) {
handleErrors(
new InvalidProjectDirectory(
'Current working directory is either not empty or not an npm project (no package.json found). Please try again in an empty directory.'
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe link to some relevant doc which explains the process e.g https://docs.coveo.com/en/cli/ or something more specific if available

@dmbrooke dmbrooke changed the title fix(cli): handle invalid directory errors when creating atomic component fix: handle invalid directory errors when creating atomic component Oct 12, 2023
@louis-bompart louis-bompart self-assigned this Nov 5, 2023
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.

4 participants