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: set default installation directory to cwd #5

Merged
merged 4 commits into from
Apr 29, 2024

Conversation

mensah-j
Copy link
Contributor

πŸ”— Linked issue

❓ Type of change

  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • πŸ‘Œ Enhancement (improving an existing functionality like performance)
  • ✨ New feature (a non-breaking change that adds functionality)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

πŸ“š Description

Fixes #4 and reverts the bug introduced in 83d09f6 (default argument set to 'process.cwd()' instead of process.cwd()).

πŸ“ Checklist

  • I have linked an issue or discussion.
  • I have updated the documentation accordingly.

@Julien-R44
Copy link
Member

Julien-R44 commented Apr 28, 2024

thanks a lot! do you mind adding a quick test for that please? 😳

@mensah-j
Copy link
Contributor Author

mensah-j commented Apr 29, 2024

Sure, no problem. However, the success condition for such a test is that japa is initialized in the current working directory. As is, this would interfere with the project directory and alter its package.json file, changing the test command and altering the project's bin/ directory. Therefore, the proposed test will need to change working directories before installing japa.

Some remarks:

  1. I believe the @args decorator is evaluated only once during the declaration of the property. This means the default value in my original PR (process.cwd()) will not update if one changes directories for this test, and should be removed in order to make the test function.
  2. There is already a check in the prepare() method which checks if destination is empty and replaces the value with cwd() (I think this could be replaced by process.cwd() for consistency)
  3. The object supplied to the decorator is missing the required: false option; unless I am mistaken, I think this is needed in order for the kernel to stop complaining that destination is missing.
  4. Since the proposed test changes working directories, the lifecycle hook should be modified to reset the current directory to the project root to prevent potential side effects.

I have added the test and changes that address the above points. Let me know if there any issues with this.

@Julien-R44
Copy link
Member

thank you so much! it's perfect dude. gonna release it

@Julien-R44 Julien-R44 merged commit 49d0cd3 into japa:develop Apr 29, 2024
6 checks passed
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.

npm init japa creates a folder named "process.cwd()"
2 participants