Ghost: Tools & Docs Improvements

[kevinansfield]

Issue Summary

Core team members and contributors have been experiencing varying levels of pain keeping Ghost up to date and with day-to-day development between server/client work. The underlying issues for this pain appear to be:

• lack of/poor documentation for day-to-day development tasks
• lack of commands that encapsulate/abstract typical workflows
• environment issues that are resulting in less than optimal development experiences
• inherent complexity of our setup that requires deeper understanding of the underlying tooling (likely tied in to the lack of documentation mentioned above)

Action Plan

• [x] improve documentation to cover day-to-day development and troubleshooting scenarios
  
  
  
  • this is particularly important for client dev that is not well documented at present
  
  
  • [x] client dev guide
  
  
  • [ ] update working-with-ghost as a top level guide (moved to #7421)
  
  
  • [ ] rework main contributing guide (moved to #7421 as optional)
  
  
• [x] split grunt dev into client and server commands
  
  
  
  • possible to run only the server reloading without client builds
  
  
  • possible to run server reloading in one tab and client builds in another, maybe useful if crashing of one or the other is likely
  
  
  • [ ] documentation to explain when/why to use the individual commands (moved to #7421)
  
  
• [x] get admin livereload into a mergeable state (https://github.com/TryGhost/Ghost/pull/8176 & https://github.com/TryGhost/Ghost-Admin/pull/590)
  
  
  
  • live reload for client builds
  
  
  • solves current client testing pain where grunt dev can't be run simultaneously with the client tests
  
  
• [x] start collecting concrete pain points from team members
  
  
  
  • it's difficult for those of us working day-to-day with Ghost to formulate plans for helpful commands at the moment as we're not experiencing the pain points ourselves, having a collection of issues will help guide documentation and tooling improvements
  
  
• [x] clean up gruntfile (moved to #7421)
• [x] npm run init on feature branches - warning? (moved to #7421)

Subtasks:

• [x] hide the npm stderr output for npm scripts so that it's easier to see actual error output and we get useful error reports, refs #8231

Potential tooling improvements

npm run dev-update
One of the noted pain points was taking an old development setup and getting it up to date with master for both client and server. For this there's a proposal to add an npm run dev-update command (naming TBD, an alternative might be npm run dev-master) that works through the commands that we're currently using to update manually:

1. git checkout master
2. git pull upstream master
3. npm install
4. cd core/client
5. git checkout master
6. git pull upstream master
7. npm install
8. bower install
9. cd ../../
10. knex-migrator health

This should initially check to see if there is an upstream remote and where it is pointing to, if it doesn't exist it should be added, if it does exist and doesn't point to Ghost's core repos then we should print an error with instructions to provide a flag or a link to tooling documentation.

The knex-migrator health command run after updating everything to master checks the database setup and will instruct you if you need to initialise the database or run migrations.

There are two proposed flags to this command based upon current team members preferred workflows:

• --upstream=theirs - for users who have a different remote name for what is traditionally called upstream
• --provider=yarn - for users who have switched from npm to yarn (this will also be at least 4x faster from initial testing)

Git hooks
We've floated the idea of using git hooks to help avoid some common pitfalls during development, so far two concrete use cases have been identified:

• running (or advising to run) npm/yarn/bower install when the package.json, yarn.lock, or bower.json files have changed after running git pull
• preventing commits to working branches that update submodule references - this could help contributors or core devs with a habit of staging all files from accidentally submitting PRs containing core/client changes (e.g. https://github.com/TryGhost/Ghost/pull/8217#issuecomment-289193299)

One problem with client-side git hooks is how to distribute them and ensure that they are installed in the project's .git/hooks directory. This needs some further research, it may require them to be distributed in a githooks directory for example and installed via one of our init scripts.

[disordinary]

(copied verbatim from slack)

I think another thing is to be extremely verbose with any messages explaining what is going on.

So when you first do npm run init it says something at the end like:

Ghost is installed and up to date with origin/master, Ghost-Admin is a git-submodule and is located in /core/client, it is also up to date with origin/master.
run grunt dev to start ghost and automatically update ghost-admin on change (slower) or grunt dev-server to start ghost without updating ghost-admin.

When a user does an update and the node version has changed it could say something like:

Ghost has binary dependencies that you installed with node 4.4.6, you're currently running node 6.10.6, please switch to node 4.4.6 or run npm rebuild

If we try and do a upgrade but you're not actually on origin/master it could say:

Your /ghost/admin is on the branch your-feature-branch-name and cannot be automatically upgraded, it is 7 commits ahead and 6 commits behind origin/master

Etc, the point is our workflows are basically the same and there will be known and common issues that we can't automatically get around without stashing or losing work, and without doing massive rebuilds, if we capture those cases and check for them we can prompt ourselves on how to fix the issues if we come back to it after working on something else for a few months.

[kevinansfield]

Issue content updated with a section on git hooks.

[ErisDS]

The points in this issue seem to cover everything, in short I think we need a 3 step approach to improving our tooling, in order of priority:

1. Improve the documentation, especially around the client side
2. Improve the tooling to solve the problems we do know about (resetting env)
3. Being diligent about writing down issues when we come across them, and reviewing those issues

When it comes to the new tool for resetting an env, it seems there is some uncertainty about what the tool should look and work like, that I think we can only get clarity on by trying a few things out.

Still I am interested to hear the reasoning behind using an npm script instead of grunt? Also - I like the idea of the flags, I think that there is a way to make npm scripts use env vars, and grunt definitely can, so perhaps that's a better option?

[kevinansfield]

Still I am interested to hear the reasoning behind using an npm script instead of grunt?

I think this mostly stemmed from looking at it from a contributors view where npm run will work even when global deps are missing so that we can output useful error messages if the environment isn't quite right. Other than that I'm not too sure - whichever route we choose between npm/grunt we still have the problem that the locally available tools are tied to the version of ghost that you currently have checked out. The only way around that I can see would be to have an external cli-like tool that can auto-update.

[disordinary]

I had a quick look around and it doesn't seem like there is a useful way to figure out the node version that a binary dependency was compiled against. Fortunately sqlite3 seems to have it in the name of the library in ./sqlite3/lib/binding/ but other binary deps don't. Seeing as sqlite3 is the most likely problem it still seems worthwhile to check that dependency within our scripts.

We can figure out the version of node that sqlite3 was compiled against from the library name, the naming convention appears to go node-{ABI}-{OS}.node. We can figure out the node version that's required from the ABI with the node-abi package.

[ErisDS]

In my opinion, grunt master should set Ghost-Admin to master, not the latest commit tied in Ghost master - i.e. it should do the same git commands, not use submodule update --init.

I have been using it since it was introduced, and keep getting confused by bugs that are fixed in master, but since the commit ref was updated.

As a dev, I always want the latest changes.

Does anyone disagree? Is it ok for me to make this change?

[kevinansfield]

I've been looking at a pre-commit hook that runs ESLint (or JSHint/JSCS for the server side) on only the changed files when committing - I thought it might help with the annoying situation where you push to a PR only to realise some time later that Travis has failed on a minor linting error.

This script looks like a good start but I think it could be adapted to have a similar yes/no approach to our current submodules script https://github.com/tryghost/ghost#ghost-10-alpha-developer-install

It _may_ be possible to do this in a pre-push hook to cut down on any local dev pain it could introduce but some more research is needed into how to grab the changed files.

Does this sound like something that would be useful? Personally I try to never commit with linting errors (Atom does a pretty good job of highlighting them) but I wonder how disruptive it could be to the workflows of everyone else.

[ErisDS]

Mebbe a pre-push rather than a precommit, but that's probably an optimisation.

One thing I can speak to - we should just have one defacto eslint ruleset that we use everywhere IMO.

[kirrg001]

We have optimised our tooling for 1.0 in this task 🎊

I've moved the last subtasks to our 1.0 documentation issue. I would like to collect all tasks for 1.0 documentation in one true place. Closing.