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.

Sign up for GitHub

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

Jump to bottom

Support globs #29

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Support globs #29

wants to merge 2 commits into from

Conversation

SimenB
Copy link
Contributor

@SimenB SimenB commented May 28, 2017 •
edited
Loading

Fixes #11

@SimenB SimenB requested review from sveisvei and ivarconr May 28, 2017 08:33
@SimenB SimenB force-pushed the glob branch 2 times, most recently from d200a6e to 6813ecc Compare May 28, 2017 10:24
@SimenB
Copy link
Contributor Author

SimenB commented May 28, 2017

This is a breaking change if you upload a directory outside of your own CWD. In that case, you also have to pass cwd. Doing cdn-uploader ./some-assets -a app still works as before.

ivarconr
ivarconr reviewed May 28, 2017
View reviewed changes
README.md Outdated
@@ -24,7 +24,13 @@ $ npm install @finn-no/cdn-uploader -g

Actual usage:
```sh-session
$ cdn-uploader /tmp/cdn-assets -a test-app
$ cdn-uploader /tmp/cdn-assets --cwd /tmp/cdn-assets -a test-app
Copy link
Contributor

@ivarconr ivarconr May 28, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

I would expect the --cwd option would imply this should be written as:
cdn-uploader ./ --cwd /tmp/cdn-assets -a test-app

ivarconr
ivarconr reviewed May 28, 2017
View reviewed changes
README.md Outdated
### Pattern
You cna pass in a pattern. E.g. to only upload javascript files:
```sh-session
$ cdn-uploader '/tmp/cdn-assets/**/*.js' -a test-app
Copy link
Contributor

Choose a reason for hiding this comment

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

not correct? pwd option or make it relative?

@ivarconr
Copy link
Contributor

ivarconr commented May 28, 2017 •
edited by sveisvei
Loading

Previously the requirement was that you had to provide a directory, with explicit feedback in the console if the provided path was not a directory. This is somewhat changed now.

What happens if you specify an invalid glob or a folder that does not exist? Will the user get any feedback at all?

Maybe there should be some explicit feedback if your pattern does not match any files?

@sveisvei
Copy link
Contributor

Indeed, should assure the command fails if nothing matches.

@SimenB SimenB force-pushed the glob branch 2 times, most recently from 4c60492 to fcdb79d Compare November 11, 2017 12:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ivarconr ivarconr ivarconr left review comments

@sveisvei sveisvei Awaiting requested review from sveisvei

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@SimenB @ivarconr @sveisvei