JedWatson / happiness

Standard customised to make me happy
MIT License
124 stars 15 forks source link

Use latest standard-engine #5

Closed dcousens closed 8 years ago

dcousens commented 9 years ago

See https://github.com/Flet/semistandard/ for reference. We need to rebase and use the latest standard-engine and eslint plugins.

wesleytodd commented 8 years ago

Hey, so totally new to this project, so please tell me if I am way off base here.

I tried updating the deps like mentioned here then looked at pulling in their version 5.4.0. You mentioned rebasing but since this repo is not a fork of standard you cant actually rebase in their updates. What was the plan here for pulling in their changes?

Any input on how you guys want this update and I am happy to help :)

wesleytodd commented 8 years ago

So I took a look through semistandard's commits and it looks more like they manually pulled over the changes from standard. Is that how you want to manage this module? Cause I can fairly easily do that since it is not much in the way of code.

wesleytodd commented 8 years ago

For reference this commit is what I mean by "manually": https://github.com/Flet/semistandard/commit/97dbfeb0ef1e44713334592a802a0297faa05c1d

wesleytodd commented 8 years ago

So totally not to steal any thunder from this project, I created a repo that does exactly this but is actually a true fork of their repo, with an intact git history which should make it possible to continuously pull in their updates: https://github.com/wesleytodd/more-happiness

I would love to work with you and make this particular implementation the correct one, but it probably needs to be restarted with the methodology I used in that repo:

git init
git remote add standard_upstream git@github.com:feross/standard.git
git fetch standard_upstream
git merge standard_upstream/master

Then when we want to update all you have to do is:

git fetch standard_upstream
git co -b merge-upstream-updates
git merge standard_upstream/master

Then test/fix and merge merge-upstream-updates into master and republish. Anyway, just wanted to post this here in case this helps.

Lastly, in my version of this I put together a script that will fix the tabs and semicolons. I ran it against the code pulled in from standard and it works pretty well. This might be a nice tool in the future for pulling things in and keeping them Happiness Style Compliant (TM pending)...

https://github.com/wesleytodd/more-happiness/blob/master/scripts/fixjs

Just needs fixmyjs installed: npm install -g fixmyjs

dcousens commented 8 years ago

@wesleytodd would you be OK if I added you as a contributor to this repository?

wesleytodd commented 8 years ago

Sure thing!

Is the change I recommended above something you guys would like to move forward with? If so, it will take me a bit, and I would do it on a branch so you all can look and make sure it is good before we overwrite master with it.

dcousens commented 8 years ago

Sounds perfect @wesleytodd, and absolutely the change is something we're interested in. @JedWatson (nor I) just haven't been able to allocate the time.

In terms of overwriting the tree, that is one option, alternatively you can just rm * the current files and replace them with your own. But, as you said, that means we can't rebase as easily so, I'm with you :+1: .

dcousens commented 8 years ago

Ping @JedWatson can you add @wesleytodd as a contributor :+1:

wesleytodd commented 8 years ago

Totally understand the time constraints! That is what Open Source is for, getting help from people who can.

Overriding the history is the only reliable way for git to correctly merge things in the future, unless you know another way? I have actually done this with a few projects before for a longer support cycle than just a few merges, and it has worked pretty well. You still get conflicts, and in our case we might get even more because tabs/semicolons are on every line, but I think a good merge/update script can be written that makes it fairly painless. Anyway, I'll take a look, probably this weekend, if you give me that commit bit :)

wesleytodd commented 8 years ago

@JedWatson & @dcousens Let me know when you get around to adding me :)

dcousens commented 8 years ago

Ping @JedWatson

JedWatson commented 8 years ago

Done! Sorry for the delay @wesleytodd, have been travelling a lot in the last few weeks, notifications got really out of hand :persevere:

Thanks for taking this on!

wesleytodd commented 8 years ago

No worries! Just was excited to start using it on my projects and didn't want to use my fork. Thanks for adding me, I will take a look at this tonight or tomorrow.

wesleytodd commented 8 years ago

https://github.com/JedWatson/happiness/tree/standard-fork

I think that covers it. If we wanted to override master with this we can just run:

$ git push origin standard-fork:master --force

Let me know what you all think :)

PS: sorry it took me this long, work has been busy and I never got around to integrating it until now

dcousens commented 8 years ago

Amazing work @wesleytodd, I'll have a look at this ASAP :+1:

dcousens commented 8 years ago

@wesleytodd is fixmyjs necessary? It seems like something you'd have globally, not part of a package. Same with win-spawn?

dcousens commented 8 years ago

Those obviously correlate with the scripts/ directory too. Was this meant to be a way to do --format?

wesleytodd commented 8 years ago

I consider it bad form to ask people to install global modules to work on a project. Npm scripts and the 'node_modules/.bin' directory solves this much better IMO. And so since the scripts require them I figure it is better to define them as dev deps.

For win spawn. I have no clue about that. I didn't add that intentionally. I'll take a look when I get home from this flight.

The scripts in there would be for pulling in new changes to reformat things to the happiness style. Is there some flag for that that I just don't know about?

wesleytodd commented 8 years ago

Can't edit posts on github mobile....

Edit: is --format a flag for standard?

wesleytodd commented 8 years ago

So it looks like win-spawn was added in the upstream: https://github.com/JedWatson/happiness/commit/1ab5b44f94c5cc86081d3642aa13e3e91b727ba9

And totally did not know this existed: https://github.com/feross/standard#is-there-an-automatic-formatter

I will switch to that which will just get rid of those scripts. Thanks for pointing that out!!

wesleytodd commented 8 years ago

Ok, so it looks like we will have to also maintain a fork of https://github.com/maxogden/standard-format to get that working. I will take a look at that tonight, shouldn't take more than a few minutes to make the changes.

dcousens commented 8 years ago

Awesome @wesleytodd! :+1:

wesleytodd commented 8 years ago

Turns out it already existed: https://www.npmjs.com/package/happiness-format

Updated!

wesleytodd commented 8 years ago

Ping :)

dcousens commented 8 years ago

Will get on to this soon :)

wesleytodd commented 8 years ago

For what its worth, we have now been using my branch of this at work, and I have been using it at home, for about two weeks now; working great. I would love to start using it in my published modules, but cannot until we get this merged and published. Sorry to be such a squeaky wheel :)

dcousens commented 8 years ago

Sorry to be such a squeaky wheel :)

Not at all. I really appreciate you pinging.
Reviewing now.

dcousens commented 8 years ago

Looks great @wesleytodd :+1: I'm going to squash https://github.com/JedWatson/happiness/commit/4cff5ecd0c57659baba9baaffa30ce20fbb2bdf5 and https://github.com/JedWatson/happiness/commit/b8e6793b2b31fb83bc8757c2f6f1eca353cc2515 then force push over the master branch.

dcousens commented 8 years ago

Backed up master to a backup branch. Force pushed over master.

@JedWatson good to publish (minor).

wesleytodd commented 8 years ago

Haha, so fast!!! I was trying to install it just to fix the errors in the project I am working on, but not actually save the dependency and it stopped working, lol...so i checked back here. Thanks!!

wesleytodd commented 8 years ago

sorry, but......ping :)

dcousens commented 8 years ago

@JedWatson!!!

edit: if he doesn't see this, I'll give him a call

wesleytodd commented 8 years ago

lol, its not a big deal, just following up, but thanks!!

wesleytodd commented 8 years ago

Merged and published!

dcousens commented 8 years ago

Woot :+1: