-
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
automate wallets version updates. #3
base: master
Are you sure you want to change the base?
Conversation
Travis CI build is failing because NPM_TOKEN environment variable is not set, and it's used in .npmrc config file that npm get i'ts config from. Two approaches:
|
@@ -39,5 +39,8 @@ | |||
"bugs": { | |||
"url": "https://github.com/LePetitBloc/wallets/issues" | |||
}, | |||
"homepage": "https://github.com/LePetitBloc/wallets#readme" | |||
"homepage": "https://github.com/LePetitBloc/wallets#readme", | |||
"dependencies": { |
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 wallets.json package should come with 0 dependencies when integrated with other projects.
At first I thought about the wallets update automation as a separate project.
But maybe it could live inside this very project, but all dependencies should be considered as dev then
|
||
class Version { | ||
constructor(versionRegexpResult) { | ||
this.prefix = versionRegexpResult[1] || ""; |
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 is a prettier in this project, all your code should be prettified. The command:
"format": "prettier --trailing-comma es5 --single-quote --print-width 120 --write \"src/**/*.js\"",
might need to be changed to include your code.
static fromVersionString(versionString) { | ||
if (typeof versionString === "string") { | ||
versionNumberRegexp.lastIndex = 0; | ||
let regexpResult = versionNumberRegexp.exec(versionString); |
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.
could be const
} | ||
|
||
toString() { | ||
let string = this.prefix + this.major + "." + this.minor; |
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.
prefer template litterals for concatenation
} | ||
|
||
toString() { | ||
return "updated " + this.walletIdentifier + " from " + this.from.toString() + " to " + this.to.toString(); |
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.
For logging purpose you should look into https://github.com/pinojs/pino
async function updateFile(updates) { | ||
updates.forEach(update => { | ||
if (update) { | ||
wallets[update.walletIdentifier].tag = update.to.toString(); |
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 should not mutate wallets variable.
You could use the spread operator for doing so:
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Spread_syntax
|
||
async function checkForUpdates(wallet, identifier) { | ||
const tags = await listRemoteTags(wallet.repository); | ||
let versions = parseVersionsTags(tags); |
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 could chain parseVersionsTags(tags).filter( ).sort(
} | ||
|
||
async function checkAllForUpdates() { | ||
const pendingUpdates = []; |
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.
Seems convoluted, even If I know we use to do things this way....
Might get something better with something like:
const pendingUpdates = Object.keys(wallets).map(key => checkForUpdates(key, wallets[key]))
```.
(a.major >= b.major && a.minor >= b.minor && a.patch >= b.patch && a.fourth > b.fourth) | ||
) { | ||
return 1; | ||
} else { |
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.
else not needed
} | ||
|
||
toString() { | ||
return "updated " + this.walletIdentifier + " from " + this.from.toString() + " to " + this.to.toString(); |
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.
again template litterals
@JulesAaelio I would love to see an
|
Description
Add a script
updater.js
that fetch version tags on wallets repository and automatically update wallets.json if necessary.Related Issue
Close #2
Motivation and Context
See #2
How Has This Been Tested?
This has been humanly tested.
Types of changes
Checklist: