-
Notifications
You must be signed in to change notification settings - Fork 6
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
S3 Without the Linter Changes #5
base: master
Are you sure you want to change the base?
Conversation
@@ -33,10 +36,12 @@ const noUpdateConfiguration = { | |||
"notes": "" | |||
} | |||
}; | |||
// AWS.config.update({region: 'REGION'}); |
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 commented code
} | ||
|
||
prefs.currentURL = tunnelURL; | ||
|
||
const currentIP = os.networkInterfaces().en0.find(elm => elm.family == 'IPv4').address; | ||
const currentIP = os.networkInterfaces().en0.find(elm => elm.family === 'IPv4').address; | ||
// const currentIP = "127.0.0.1";// os.networkInterfaces().en0.find(elm => elm.family === 'IPv4').address; |
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 commented code.
Bucket: bucketName, | ||
Key: fileName, // File name you want to save as in S3 | ||
Body: fileContent, | ||
Acl: "public-read" |
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.
Should we make this ACL configurable?
await checkForUpdate(preferences); | ||
if (options.delete_apk.toString() === "1") { fs.unlink(pathToFile, () => { }) } |
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.
What's this?
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.
to remove the local build file, if the user wants.
.alias('a') | ||
.option('-b, --branch <branch>', 'The branch the build is from') | ||
.option('-n, --notes <notes>', 'Release Notes for the build') | ||
.option('-d, --delete_apk <delete_apk>', 'Release Notes for the build') |
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.
The argument and comment mismatch. Also, this flag seems to be platform specific (Android)
}; | ||
|
||
program | ||
.command('submitS3 <pathToFile>') |
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.
How does this new command work. Is the person supposed to call librarian submit
, or submitS3
?
Instead of duplicating all of the submit code, we can also add a flag to existing submit code to upload to S3? Something like librarian submit --s3
{ | ||
type: 'confirm', | ||
name: 'enable_s3', | ||
message: 'Do you want to enable s3 storage?', |
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.
Can we add some information to the README regarding this new feature?
No description provided.