Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Updates to goog.module and ES2015 #6

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Updates to goog.module and ES2015 #6

wants to merge 2 commits into from

Conversation

itsthesteve
Copy link

  • Bump Angular version to 1.6.1
  • Leverages goog.module and ES2015
  • Version pin closure sources
  • Boilerplate samples

Uses goog.module style based on go/js.
a84161c 
- Leverages goog.module
- Sample boilerplate
- Bump Angular version to 1.6.1
- Version pin closure sources
- ES2015 syntax
@jeremydw
Copy link
Member

Looks great - thanks! Since this scaffold is so opinionated, I'm thinking that we're going to need to rename it to something like base-closure-angular after we merge this in (base is our new barebones scaffold).

Everything looks pretty good but I did want to note that it assumes that the user has nvm installed which I know not everyone does. Can you either add a Makefile that allows the user to automatically install nvm by running make develop or strike the nvm stuff for now?

drGrove
drGrove reviewed Jul 21, 2017
View reviewed changes
.travis.yml
- 2.7
- 2.7
node_js:
- "6.2"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should bump this to 6.9.5

.travis.yml
cache: pip
before_install:
- nvm install 6.2
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

6.9.5

.travis.yml
- npm install -g gulp
- npm install -g bower
- pip install grow
- nvm use 6.2
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

6.9.5

package.json
@@ -1,16 +1,21 @@
{
"name": "scaffold",
"private": true,
"engines": {
"node": "^6.2.0"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

6.9.5

package.json
@@ -1,16 +1,21 @@
{
"name": "scaffold",
"private": true,
"engines": {
"node": "^6.2.0"
},
"devDependencies": {
"bower": "^1.3.12",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is more of a personal preference, but I'd pin versions

podspec.yaml
@@ -1,11 +1,10 @@
grow_version: ">=0.0.60"
grow_version: ">=0.0.67"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

0.2.0?

.idea/vcs.xml
@@ -0,0 +1,6 @@
<?xml version="1.0" encoding="UTF-8"?>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Drop file

@jeremydw
Copy link
Member

We should also look at growthemes/base for a decent-looking more minimal theme. If you want to create an opinionated theme, that is totally OK - we might want to start it as a new repo, though. Let me know and we can create one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@drGrove drGrove drGrove left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@itsthesteve @jeremydw @drGrove