waldyrious / primerpedia

Simplified extracts of Wikipedia articles, showing just the basic information.
https://primerpedia.toolforge.org
Other
11 stars 7 forks source link

Linting #32

Closed Jan-Ka closed 6 years ago

Jan-Ka commented 6 years ago

Just adding some Housekeeping Stuff. The .eslint config matches the .editorconfig.

However I would propose switching from tabs to spaces.

Edit: Forgot about index.html

While it is standard compliant to use single quotes in (X)HTML it's still a bad idea and only causes confusion.

waldyrious commented 6 years ago

I agree with changing tabs to spaces. It's less semantically meaningful, but less prone to mishaps in the long run. Feel free to change it in this PR if you like.

Jan-Ka commented 6 years ago

I hope the order of the commits makes it easier to see what the real changes are.

f7e9aca51308a9d17f77e4db30de2d730555c374 might be a bit controversial since it introduces opinionated workspace settings for visual studio code users but otherwise everyone (e.g. me) pressing format on the file will break the agreed upon formatting. VScode does not support the required options for it's HTML formatter to keep it the way we want.

I think this is a agreeable piece of convenience which is easily changed when the mob starts banging it's pitchforks and torches against the main gate.

waldyrious commented 6 years ago

This looks good to me, nice work :+1:

Before merging, however, I would like to clean up the branch history, to avoid the merge commit in there. I don't mind wrangling with git, so if that's not your cup of tea, let me know and I'll take care of it :)

Jan-Ka commented 6 years ago

Would be nice if you could take care of that. Thank you very much.

waldyrious commented 6 years ago

Ugh, we're getting conflicts already. Let's get this done before we create too much of a mess.

waldyrious commented 6 years ago

I've rebased the branch to (1) put it on top of the latest gh-pages, (2) remove the merge commit 7aa08ac and the fixup commit facdd45 (by integrating their changes from the beginning), and (3) split the single quotes --> double quotes commit into 3 atomic changes (so that later rebases may happen more cleanly).

I should have reused e2c5ca2 instead of committing fa36b68 from scratch; I might do that next time I rebase this branch.

I won't merge this now since there are various other PRs in progress, and I don't want to make things harder for all of them. Once things stabilize a bit I'll re-rebase this and merge it.

Jan-Ka commented 6 years ago

Hey @waldyrious , I took the liberty to create a label for PRs that are "on hold" for merging because of any given reasons just so it's easier to see at a glance in the PR list.

Jan-Ka commented 6 years ago

Hey @waldyrious, Maybe we should configure this repositories project to make it more explicit when things are going to get merged?

Also does this block #39 ?

waldyrious commented 6 years ago

Maybe we should configure this repositories project to make it more explicit when things are going to get merged?

No objections. How do you suggest we do that?

Also does this block #39 ?

Not strictly, but I wouldn't want to set it up and have the human effort done here go to waste. That said, I'll defer to your choice, of course.

Jan-Ka commented 6 years ago

I took the liberty to create #55 to discuss the Project setup in.

As to the loss of human effort. I can of course only speak for mine but I honestly do not care since automatic linting is preferable and should lead to the same results anyway. Only "real" work done is in creating the configuration files.

waldyrious commented 6 years ago

Understood. I'll see if I can integtate this in the next couple days, now that #41 is merged; otherwise let's move on with the linter.