Automating Build Process

[p1ho]

I don't know if this is necessary, but I think it's somewhat redundant to ask everyone who submits a PR to also submit the minified code.

We could implement a build process, where we ask people to make PR to a review branch. If it's approved and merged, it will activate a build process somewhere else (using tools like Travis CI) and push the minified version to the master branch if it passed all tests (which is currently being discussed #21)

The good thing is, this will maintain backwards compatibility as it does not change the githack URL for the minified file that's being imported by all current users.

[oscarmorrison]

Yep. I agree @p1ho

PR welcome

[oscarmorrison]

Using Github CI might be a good option: https://github.blog/2019-08-08-github-actions-now-supports-ci-cd/

Otherwise travis has always been good and simple from my experience.

[p1ho]

I had always wanted to give GitHub CI a shot, I'd say let's go with that for now. I can try to set up something really basic and create a PR, and we can see if the merging process is painless.

[oscarmorrison]

Sounds good. Thanks @p1ho

[p1ho]

@oscarmorrison Exciting news, I got a proof of concept working!

It was a lot of trial error, so I did all of these in a separate test repository. It's my first time using GitHub action so I'm sure we can optimize the workflow script better.

The idea is as follows:

• Push/PR action happens on the review branch
• GitHub action executes release.yml
• test and build (will stop if fails)
• if passes:
  • checkout local master branch
  • run build again
  • commit built md-page.js (not gitignored)
  • push to remote master branch.

In summary, even though md-page.js is not gitignored, it will not be part of the review branch, but will only be on master branch. This means contributors don't have to concern themselves with md-page.js at all (since it's not human readable anyways). However, as reviewers, it'll be on us to make sure md-page.js is not submitted in any future PR.

Because this may be repo breaking, I'll only submit a PR once everything is finalized, and there would need to be a branch for reviewing on this repo (can be named anything)

Relevant Links:

• master branch
• review branch
• build history on test repo (very brutal)

Edit: Fixed links

[oscarmorrison]

Excellent work @p1ho. I have created a new review branch https://github.com/oscarmorrison/md-page/branches

Not sure on the plan here, but we can either keep the make file, or get rid of it all together and just use npm. Happy with either.

[p1ho]

Great, I'm still finalizing things, I'll make a PR when I'm ready.

In my new repo, I've moved the 2 commands in makefile to package.json's script section. I think it's cleaner this way. Ideally you'd want package.json anyway since you have a dev dependency on uglify.js, I personally don't think it makes sense to force developers to install uglify.js globally.

[oscarmorrison]

Yep. I agree. When I built this, didn't plan on releasing it publicly. Hence the makefile, static dep. Are you going to make uglify dep as part of the PR?

[p1ho]

Yes, please see test repo for proposed folder structure. I'll also add a dev section to the README with the PR.

Proposed workflow for developers:

• clone repo
• run npm install to install dev dep (jest and uglify)
• make changes to the 2 source files in /src
• test their changes using index.html (the links will have to be fixed)
• if necessary, write tests in /test

[oscarmorrison]

Gotcha. Sorry didn't look at repo before. Looking good. Fine work @p1ho

I'll also add a dev section to the README with the PR.

I would prefer keep this in here https://github.com/oscarmorrison/md-page/blob/review/DEV_README.md

I want to keep the readme (as its the landing page) as simple as possible). Majority of people will just be copying and pasting the script tag, and getting up and running

[p1ho]

That sounds good too, I'll put those information in DEV_README.md instead.

[p1ho]

Just to set an expectation for when I'll open the PR. Push is straightforward, but I'm still working on the trigger timing for pull requests, I'll likely open the PR at the latest this weekend.

[oscarmorrison]

Sounds good.

[p1ho]

Here's something UX related we should think about.

Maybe it's worth considering making review branch the default branch, so that when new devs come along and they clone the project, they would end up with a repo without the built file (?) This should also make opening PR slightly easier since the review branch would be the default one.

I was thinking this because I had to manually change the branch every time i open a PR in my own test repo and it was a bit frustrating.

[oscarmorrison]

@p1ho great minds thinks alike. I already changed default branch to review

[p1ho]

Opened PR at #42

[oscarmorrison]

Closed by #42