-
Notifications
You must be signed in to change notification settings - Fork 1
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
Support file path glob pattern #5
Conversation
43472a1
to
f548d0b
Compare
f548d0b
to
d3132e6
Compare
Main file to review is main.ts |
d391c5e
to
0ddbc5e
Compare
0ddbc5e
to
d24b82f
Compare
} | ||
console.log("Found files matching path: " + input_binary_path); | ||
console.log(files); | ||
// Upload all the files that matched the file path |
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.
I think this could be a problem (uploading multiple files), but not sure?
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.
Maybe we should add a limit of 2 or 3?
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.
Uploading only 2 or 3 out of (many) is not ideal because there is no way to control which of the (many) files get uploaded.
I think we should either error out if more than one file is matched, or upload 'em all. I vote to leave it as is. That way it is possible to build multiple apps off the same code base and upload them all with one call to the plugin.
} | ||
// retry upload 3 times |
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.
There are two instances of this code in the PR (the "retry upload 3 times" logic); we should only have one, I think?
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.
you were confused i think by the fact that there is a ts and js file. However it is only present once in the TS file (and a second time on the generated js code)
main.ts
Outdated
"Did not find any files that match path:" + input_binary_path | ||
); | ||
} | ||
console.log("Found files matching path: " + input_binary_path); |
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.
remove log message
Make the
UPLOAD_BINARY_PATH
input support path globs, like./app/build/outputs/apk/debug/*.apk