Add uphold-scripts project by rplopes · Pull Request #2 · uphold/uphold-scripts

rplopes · 2018-07-13T16:18:37Z

Set of scripts and dev dependencies to help with common tasks on Uphold projects.

Available Scripts:

changelog - generate changelog file for a given version
lint - run eslint to lint JS files
release - cut a new release for a new version
test - run the test suite with jest pre-configured
version - tag a new version and generate its changelog

Americas · 2018-07-16T10:55:41Z

+
+set -e
+
+case "$1" in


WDYT about adding a release script?

release) npm version $2 -m "Release %s" ;;

Americas · 2018-07-16T10:56:07Z

+
+case "$1" in
+  changelog)
+    ./node_modules/.bin/github-changelog-generator --future-release=v$2 > CHANGELOG.md


Remove ./node_modules/.bin/?

Americas · 2018-07-16T10:58:41Z

+  "author": "Ricardo Lopes",
+  "license": "MIT",
+  "precommit.silent": true,
+  "dependencies": {


Should they be fixed to a specific version? To ensure the same versions in all our projects.

AFAIK the rule is to fix specific versions on applications and to allow updates on packages. Then, any application using a package can fix the dependencies it needs.

I don't think there's any problem with project foo having resolved jest to 23.4.0 and project bar having resolved it to 23.4.1, since they both can fix a specific version if needed and they both share a common dependency that shares maintenance work anyway.

Good enough for me.

Americas · 2018-07-16T11:15:15Z

+  "name": "uphold-scripts",
+  "version": "0.0.1",
+  "description": "Set of scripts and dev dependencies to help with common tasks on Uphold projects",
+  "bin": "./bin/uphold-scripts",


WDYT about separating each command into its own file?

It started with separate files, but there were problems with loading them (they don't get added to node_modules/.bin, as the main one does). Since each of them is a one liner, didn't want to spend too much time trying to get a loader that worked.

We will eventually run into this problem, I can foresee bigger and more complex scripts popping up, and this setup won't be practical.

Hmm, tested separating and the scripts were added to node_modules/.bin just fine..

Weird. Did you test from a different project, like the huobi-client example linked in the description?

Yes, that is how I tested it. Required it from the huobi-client and I set up the scripts, and tested running each script.

Added a new commit that separates into different files. Got the same issue as before:

$ git df diff --git i/yarn.lock w/yarn.lock index 2124b3e..da63491 100644 --- i/yarn.lock +++ w/yarn.lock @@ -3685,7 +3685,7 @@ unset-value@^1.0.0: "uphold-scripts@git+ssh://git@github.com/uphold/uphold-scripts#feature/add-uphold-scripts": version "0.0.1" - resolved "git+ssh://git@github.com/uphold/uphold-scripts#6da4033bfc0bf409e56ac22c5ef2cf41f7f44c05" + resolved "git+ssh://git@github.com/uphold/uphold-scripts#83b4d40f9fdf4369d033432858809afe9d6ef0b3" dependencies: "@uphold/github-changelog-generator" "^0.7.0" eslint "^5.1.0" $ yarn lint yarn run v1.7.0 $ uphold-scripts lint sh: /Users/ricardo/Projects/uphold/huobi-client/node_modules/.bin/uphold-scripts-help: No such file or directory error Command failed with exit code 127. info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command. $ ls -la node_modules/.bin/ | grep uphold-scripts lrwxr-xr-x 1 ricardo staff 36 Jul 16 15:44 uphold-scripts -> ../uphold-scripts/bin/uphold-scripts

"bin": { "us-lint": "./bin/uphold-scripts-lint" ... }

Not a big fan of having to expose n+1 bins like that. But if it's the best option we've got...

kurayama · 2018-07-16T13:54:15Z

@@ -0,0 +1,8 @@
+{
+  "collectCoverage": true,


add clearMocks too?

Not required I think, restoreMocks does all the needed work.

kurayama · 2018-07-16T13:58:06Z

+    "src/**/*.js"
+  ],
+  "restoreMocks": true,
+  "testEnvironment": "node"


Should we add things like:

"modulePaths": ["<rootDir>"]

and

"testRegex": "test/.*\\.test.js$"

Or are these too opinionated?

Our test suites seem to run fine without them, so at least for now I don't think we really need them.

module path allows us to refer to a file as 'src/foobar' instead of '../../../src/foobar'

Ok, that sounds like a useful addition. Test regex seems to be the opposite (limiting possibilities), so maybe we can use the former and leave out the latter.

kurayama · 2018-07-16T14:49:46Z

@@ -0,0 +1,11 @@
+#!/usr/bin/env sh
+
+echo 'uphold-scripts [command] [options]';


use cat with heredoc?

cat <<EOF uphold-scripts [command] [options] Commands: changelog [version] Generate changelog file help Display this help section lint Lint JS files release Cut a new release test Run the test suite version Tag a new version EOF

Americas · 2018-07-16T15:30:15Z

+  "version": "0.0.1",
+  "description": "Set of scripts and dev dependencies to help with common tasks on Uphold projects",
+  "bin": {
+    "uphold-scripts": "./bin/uphold-scripts",


This and the help bin are a bit too much.

Not sure I agree. I'd much prefer having a uphold-scripts bin with specific commands than n different bins to memorise individually. Also, when requiring a uphold-scripts package, makes sense to then call uphold-scripts command instead of uphold-scripts-command IMO.

I was not assuming the purpose of this repo was to memorize commands. I thought they would be added automatically in the future.

I've been thinking about this, and I believe it really is better to have these scripts under one executable instead of many.

This way, we could also provide many developer utility commands without polluting the package.json, like uphold-scripts prs to open the github PRs page for that project, for instance.

That being said, WDYT about this alternative strategy:

keeping all scripts in separate files, for maintainability

having a build step on release that builds a final bin with all scripts included (similar to what webpack would do, for instance)

exporting that automatically-generated bin only, that projects and developers could call without having to load different bins at runtime

Americas · 2018-07-31T12:57:05Z

+program
+  .command('changelog [version]', 'Generate changelog file')
+  .command('lint', 'Lint JS files')
+  .command('release', 'Cut a new release')


release [version]

rplopes self-assigned this Jul 13, 2018

rplopes force-pushed the feature/add-uphold-scripts branch from 9e8fdbc to 0a515d2 Compare July 13, 2018 17:00

rplopes requested review from Americas, franciscocardoso and kurayama July 13, 2018 17:24

Americas requested changes Jul 16, 2018

View reviewed changes

rplopes force-pushed the feature/add-uphold-scripts branch from 0a515d2 to 961f0df Compare July 16, 2018 11:14

Americas reviewed Jul 16, 2018

View reviewed changes

rplopes force-pushed the feature/add-uphold-scripts branch from 961f0df to 6da4033 Compare July 16, 2018 11:16

kurayama reviewed Jul 16, 2018

View reviewed changes

rplopes force-pushed the feature/add-uphold-scripts branch 2 times, most recently from a6ea8ef to b8c2f69 Compare July 16, 2018 14:21

kurayama reviewed Jul 16, 2018

View reviewed changes

rplopes force-pushed the feature/add-uphold-scripts branch 2 times, most recently from 15a074f to d6733c6 Compare July 16, 2018 15:07

Americas reviewed Jul 16, 2018

View reviewed changes

rplopes force-pushed the feature/add-uphold-scripts branch 4 times, most recently from ce25733 to 88f6c01 Compare July 16, 2018 15:48

rplopes force-pushed the feature/add-uphold-scripts branch from 9616584 to ce56e3c Compare July 31, 2018 11:35

Americas reviewed Jul 31, 2018

View reviewed changes

rplopes force-pushed the feature/add-uphold-scripts branch from ce56e3c to e4b65d0 Compare July 31, 2018 13:29

Add @uphold/scripts project

ee3b670

rplopes force-pushed the feature/add-uphold-scripts branch from e4b65d0 to ee3b670 Compare July 31, 2018 13:31

Americas approved these changes Jul 31, 2018

View reviewed changes

rplopes changed the title ~~Add @uphold/scripts project~~ Add uphold-scripts project Jul 31, 2018

Americas merged commit e24f598 into master Jul 31, 2018

Americas deleted the feature/add-uphold-scripts branch July 31, 2018 13:36

		@@ -0,0 +1,11 @@
		#!/usr/bin/env sh

		echo 'uphold-scripts [command] [options]';

Conversation

rplopes commented Jul 13, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rplopes Jul 16, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

rplopes commented Jul 13, 2018 •

edited

Loading

rplopes Jul 16, 2018 •

edited

Loading