-
Notifications
You must be signed in to change notification settings - Fork 407
first commit #339
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
base: master
Are you sure you want to change the base?
first commit #339
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| name: Test | ||
|
|
||
| on: | ||
| pull_request: | ||
| branches: [ master ] | ||
|
|
||
| jobs: | ||
| build: | ||
|
|
||
| runs-on: ubuntu-latest | ||
|
|
||
| strategy: | ||
| matrix: | ||
| node-version: [20.x] | ||
|
|
||
| steps: | ||
| - uses: actions/checkout@v2 | ||
| - name: Use Node.js ${{ matrix.node-version }} | ||
| uses: actions/setup-node@v1 | ||
| with: | ||
| node-version: ${{ matrix.node-version }} | ||
| - run: npm install | ||
| - run: npm test |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1,53 @@ | ||
| // write code here | ||
| /* eslint-disable no-console */ | ||
|
|
||
| const fs = require('fs'); | ||
| const path = require('path'); | ||
|
|
||
| const [, , srcArg, destArg] = process.argv; | ||
|
|
||
| if (!srcArg || !destArg) { | ||
| console.error('Two arguments is required'); | ||
| process.exit(0); | ||
| } | ||
|
|
||
| const src = path.resolve(srcArg); | ||
| const dest = path.resolve(destArg); | ||
|
|
||
| if (!fs.existsSync(src)) { | ||
| console.error('Source does not exist'); | ||
| process.exit(0); | ||
| } | ||
|
|
||
| if (src === dest) { | ||
| process.exit(0); | ||
| } | ||
|
|
||
| const srcStat = fs.statSync(src); | ||
|
|
||
| if (srcStat.isDirectory()) { | ||
| console.error('Source must be a file'); | ||
| process.exit(0); | ||
| } | ||
|
|
||
| if (destArg.endsWith('/') || destArg.endsWith('\\')) { | ||
| if (fs.existsSync(dest) && fs.statSync(dest).isDirectory()) { | ||
| fs.renameSync(src, path.join(dest, path.basename(src))); | ||
|
|
||
| process.exit(0); | ||
| } | ||
|
|
||
| console.error('IT / is in the then it should be folder'); | ||
|
|
||
| process.exit(0); | ||
|
Comment on lines
33
to
41
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's great that you've added the check for destination paths ending with a slash, as requested in the previous review. However, this logic block doesn't quite work as intended. Currently, after a successful file move on line 34, the code continues execution and incorrectly prints an error message on line 37. A successful operation should exit cleanly without any errors. To fix this, you should restructure this |
||
| } else if (fs.existsSync(dest) && fs.statSync(dest).isDirectory()) { | ||
| fs.renameSync(src, path.join(dest, path.basename(src))); | ||
| } else { | ||
| const destDir = path.dirname(dest); | ||
|
|
||
| if (!fs.existsSync(destDir)) { | ||
| console.error('Destination directory does not exist'); | ||
| process.exit(0); | ||
| } | ||
|
|
||
| fs.renameSync(src, dest); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| fasdf |
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.
This error message is not very clear. According to the requirements, an error should be thrown if the destination directory does not exist. A more descriptive message, similar to the one on line 48, would be better. For example:
Destination directory does not exist.