Url Module: Bootstrap the repository and add the first module #1

youknowriad · 2017-06-27T16:30:57Z

This PR tries to bootstrap the repository using a simple Lerna config.
This PR is highly inspired by how the Jest Repository works.
The idea is trying to build our workflow around a simple module breaking things up into several PRs.
For this initial PR, I'm setting up the build script. (Trying to keep it as small as possible)
Follow-up PRs should think about linting, testing, watching...

Included in this PR:

The url module as as an example module. The example function here is actually extracted from The Gutenberg repository.
A build script allowing us to build all the modules with the same script (avoid duplicating the logic in all submodules)
The babel config is the one used for Gutenberg

Not included yet:

The browser build should automatically generate the wp.url global variable.

Testing instructions

npm install --global lerna@^2.0.0-beta.0
npm install
lerna bootstrap
npm run build

rmccue

Few indentation issues here.

rmccue · 2017-06-28T02:26:16Z

lerna.json

+    "packages/*"
+  ],
+  "version": "0.0.0"
+}


Should we use tabs in JSON config too? 🤔

Yes, I'll add an .editorconfig shortly, WordPress is switching back to tabs for .json files:

See #WP40946 and WordPress/gutenberg#1067

This is auto-generated by lerna, I wonder if it forces spacing indentation if I replace with tabs. I'll test.

rmccue · 2017-06-28T02:26:36Z

packages/url/package.json

+  "description": "WordPress URL utilities",
+  "main": "build/index.js",
+	"module": "build-module/index.js",
+	"browser": "build-browser/index.js",


Mixed indentation.

rmccue · 2017-06-28T02:28:31Z

scripts/build.js

+ * Babel Configuration
+ */
+const babelDefaultConfig = JSON.parse(
+  fs.readFileSync( path.resolve( __dirname, '..', '.babelrc' ), 'utf8' )


Mixed indentation.

rmccue · 2017-06-28T02:29:27Z

scripts/watch.js

+    return fs.statSync(filename).isFile();
+  } catch (e) {}
+  return false;
+};


Should probably tabify this file too.

youknowriad · 2017-06-28T09:01:00Z

I'll fix those indentation issues, hopefully we could get eslint soon :)

Thanks for the review, The indentation issues should be fixed. I am using tabs for everything except the package.json

ntwb · 2017-06-28T10:00:32Z

@youknowriad I'll have an ESLint PR soon 😄

youknowriad · 2017-06-28T15:25:46Z

I'd appreciate some 👀 and a ✅ to move forward with the other steps (testing, watching...)

atimmer · 2017-06-28T20:38:32Z

.babelrc

+					"last 2 iOS versions",
+					"last 1 Android version",
+					"last 1 ChromeAndroid version",
+					"ie 11"


Did you omit the last 2 edge versions on purpose?

I've just copy/pasted from Gutenberg, we missed Edge :)

atimmer · 2017-06-28T20:39:56Z

package.json

+    "mkdirp": "^0.5.1"
+  },
+  "scripts": {
+    "build-clean": "rm -rf ./packages/*/build ./packages/*/build-es5",


This won't work on windows. Is that a concern? If so, we could use rimraf.

Good catch, let's not leave Windows users behind :)

atimmer · 2017-06-28T20:44:56Z

I have commented in two locations. I have also tested this locally, for me this PR is ✅. Concerns could be addressed in new PRs.

youknowriad · 2017-06-28T21:44:51Z

Thanks for the review, Feedback addressed.

gziolo · 2017-07-02T03:15:25Z

Cool, lerna seems like the best way to go here 👍

youknowriad self-assigned this Jun 27, 2017

youknowriad requested review from rmccue, mtias and adamsilverstein June 27, 2017 16:30

rmccue previously requested changes Jun 28, 2017

View reviewed changes

youknowriad force-pushed the add/url-module branch from 99db7aa to 6914067 Compare June 28, 2017 09:59

youknowriad added 2 commits June 28, 2017 11:01

Url Module: Bootstrap the repository and add the first module

0391842

chore: Fix mixed indentation issues and drop the watch script for now

fde7ea5

youknowriad force-pushed the add/url-module branch from 6914067 to fde7ea5 Compare June 28, 2017 10:01

ntwb mentioned this pull request Jun 28, 2017

chore: First pass adding ESLint. #7

Closed

atimmer reviewed Jun 28, 2017

View reviewed changes

Chore: Using rimraf and adding Edge config for babel-env-preset

56266e2

youknowriad merged commit 736c86e into master Jun 28, 2017

youknowriad deleted the add/url-module branch June 28, 2017 21:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Url Module: Bootstrap the repository and add the first module #1

Url Module: Bootstrap the repository and add the first module #1

youknowriad commented Jun 27, 2017

rmccue left a comment

rmccue Jun 28, 2017

ntwb Jun 28, 2017

youknowriad Jun 28, 2017

rmccue Jun 28, 2017

rmccue Jun 28, 2017

rmccue Jun 28, 2017

youknowriad commented Jun 28, 2017

ntwb commented Jun 28, 2017

youknowriad commented Jun 28, 2017

atimmer Jun 28, 2017

youknowriad Jun 28, 2017

atimmer Jun 28, 2017

youknowriad Jun 28, 2017

atimmer commented Jun 28, 2017

youknowriad commented Jun 28, 2017

gziolo commented Jul 2, 2017

Url Module: Bootstrap the repository and add the first module #1

Url Module: Bootstrap the repository and add the first module #1

Conversation

youknowriad commented Jun 27, 2017

rmccue left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

youknowriad commented Jun 28, 2017

ntwb commented Jun 28, 2017

youknowriad commented Jun 28, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

atimmer commented Jun 28, 2017

youknowriad commented Jun 28, 2017

gziolo commented Jul 2, 2017