search

bocoup / blocks-capacity-planner

An airtable block for matching supply to demand in an airtable base.
MIT License
3 stars 3 forks source link

Normalize indentation #27

Closed jugglinmike closed 4 years ago

jugglinmike commented 4 years ago

Consistently use "hard" tabs for indentation, and update the linter to enforce this style.

@rmeritz Would you mind reviewing this?

rmeritz commented 4 years ago

@jugglinmike - Normally if I were to change a style guideline I would just change the guideline and then in the future, I would slowly change things as I ran into them in order to try and preserve the git history. Do you normally take the approach of fixing them in one swoop? Have you not had issues where it makes history more frustrating to navigate?

Also, is there a standard Bocoup style guide that we use in all our own projects? I understand sometimes needing to use the client one in client projects, but I feel like there should be a standard one otherwise. Though I guess in the case the eslint file came from the block init.

Anyway, this is totally fine with me to merge. I just thought while I was thinking about it I would ask Bocoups style guide philosophy (if one exists).

jugglinmike commented 4 years ago

Normally if I were to change a style guideline I would just change the guideline and then in the future, I would slowly change things as I ran into them in order to try and preserve the git history. Do you normally take the approach of fixing them in one swoop? Have you not had issues where it makes history more frustrating to navigate?

You're right: a change like this definitely interferes with git blame. That would be more of a concern if this project had a long history with multiple contributors. I'm suggesting we address this inconsistency immediately because that's not the case here.

The downside to the incremental approach is that it adds noise to future commits, and it's difficult to communicate to contributors. For instance, I didn't feel it was appropriate to ask you to adhere to a undocumented stylistic constraint in your most recent patch.

Also, is there a standard Bocoup style guide that we use in all our own projects? I understand sometimes needing to use the client one in client projects, but I feel like there should be a standard one otherwise.

Right again, and I'm glad you mentioned this. I had somehow convinced myself that Airtable uses hard tabs, but that's not the case. I've updated the patch to use their convention of 4-space tabs.

rmeritz commented 4 years ago

@jugglinmike - Thanks for providing more context to your approach. LGTM.