Enabling AirBnB lint, folder reorganization, webpack configuration enhanced, empowered npm scripts and TEMPO function

thefzn commented 5 years ago

This is a followup to the failed pull request.

Same updates, bugs fixed, latest updates integrated. Although, MongoDB is crashing while saving... even before integrating with my dev server.

Cheers!

jaythomas commented 5 years ago

Can this PR be broken down into more logical chunks? It's a bit overwhelming to review everything and the sheer amount of changes is making it fragile and prone to merge conflicts.

thefzn commented 5 years ago

Sorry, I got a bit too excited about this, here's a detailed explanation about every commit:

Code Reorganization

567a944, a3ab7de and 380e9f3 are no significant changes, just organizing the code in folders:

/src the app's code
/packing Webpack and server stuff
/dist replaces /public and it's now generated by npm script instead of holding mixed content (generated and non-generated files). Source files are located on /src/static

6c39ea5 I noticed slang.ohm and slang-grammar.js are the same file, I guess you use the OHM file to edit, then copy the changes to the JS file to import them with Webpack. Well, there's a plugin we can use to import the OHM file as a string. No duplication needed anymore.

A new 'tempo' function

97e2bb5 That's my first contribution: a new 'tempo' function, so we can set the bpm. This was previously a fixed value of 120 bpm. It affects all of the following play functions so we can do this:

TEMPO 120
play @synth1

TEMPO 240
play @synth2

AirBnB Linting

b552b62 and 6d0771c I noticed you have AirBnB Lint on your package.json but it was not being imported on your .eslint file, these commits include the import and a lot of linting.

Enpowering NPM Scripts

6d30086 There were just 2 npm scripts on the package.json: Webpack compile/watch and productive build. I guess you run a custom server locally to test and dev, so I added some commands to start an express server, lint, Webpack compile and watch... everything with npm start. Also, each step is executed on it's own command, so you can run them independently.

6c5967c /dist is now generated at npm start, we don't need the folder on the repo anymore. all of the source files are now placed on /src

Fixing bugs generated by the Lint

1652816, dfd32d7 and c15f9f2 As you may know, AirBnB Lint is very strict, there were a few errors caused by my initial lint (sorry about that), these 3 commits just resolve the previous issues.

Express/Mongo features

9e58339 While I was working, there was a new commit on the main repo, adding save features using express and mongodb. This commit is just a copy-paste of those files. I didn't have time to integrate the changes properly.

85d3626 and d044fe8 Now I've got some time... as I already had an express server that runs (and watches) the compiled Webpack scripts, I just needed to integrate the save functions on that server... also, added a few improvements:

Saved patches has an expiration date of 2 days. Each request (save or load) will cleanup expired patches first.
You were using a truncated random number as patch identifier, I changed the function and now produces unique encoded hashes similar to the ones generated by goo.gl/HASH or bit.ly/HASH (Unix date in milliseconds encoded)
As the server is now fulfilling a different function (not just packing and testing), I created a new /server folder to contain Express and MongoDB setup.

Still pending

I still have pending to organize dependencies and devDependencies on the package.json, if I recall right, express and its tools are devDependencies on my repo... I thought the Slang Language was the final product, but I'm starting to think it's the editor, so they may need to be listed as dependency.

kylestetz commented 5 years ago

Hey @thefzn, sorry for the delay, I didn't expect this project to have quite so much visibility 😅

I really appreciate your enthusiasm and there are some cool ideas here, but this is too much code for me to consider merging. I also want to apologize, I intend to put a contributions section in the readme with some guidelines for how I'm interested in getting folks to contribute, but I haven't had a chance to do that yet. I also hope you understand that, more than anything else, this is a creative project for me so I might veto features that I'm not as interested in.

I really like pulling the Ohm file in using a webpack plugin, and I like the idea of a tempo syntax, generally speaking. Separately from this PR I'd want to think through the tempo syntax itself and see if there are other options that have different affordances. Basically I want to understand how different options would impact the experience of making music in Slang, which keeps in line with how I've been approaching this project.

On the other hand, I'm not interested in rearranging all of the code 😄. The way you've organized it is just as opinionated as the way I've organized it; this is how most of my projects are set up right now, and part of that has to do with how my VPS is set up with nginx, etc. Just personal preference.

All of that said, you are an absolute machine and I don't want to stop you from having fun with this, so I'd encourage you to keep going on your own fork, see where it takes you, and share back whatever wild stuff you come up with!

As far as contributing goes, it would be great to move some discussion into issues — maybe one for the tempo syntax, to start. I would also definitely accept 6c39ea5 as its own PR.

Cheers 🍻

thefzn commented 5 years ago

I'll definitively continue this project on my fork but I also want to contribute!

Everything sounds reasonable, I'm still very excited about this! I'll create a separate fork to contribute here as soon as I have some free time.

By the way, this project was featured at October 2nd's issue on this eMagazine called eWebDesign.

kylestetz / slang