multitheftauto / mtasa-resources

This project maintains a list of up-to-date resources that come with Multi Theft Auto.
https://multitheftauto.com
MIT License
151 stars 151 forks source link

Establish & enforce style rules #250

Open jlillis opened 3 years ago

jlillis commented 3 years ago

I think it would be a good idea to establish and begin enforcing (via code reviews and CI checks) style rules for this repository just like we do for mtasa-blue. To this end, I have submitted a PR that adds a .editorconfig file that defines the following basic rules:

Line endings

There has been some inconsistency regarding which type of line endings are meant to be used. The .gitattributes file calls for LF endings and it's been mentioned that in the long-term we want LF, but a recent commit seems to have standardized on CRLF. I guess deciding on this would be the first order of buisness.

CI checks

On CI, there have been previous attempts to use lua-fmt to automate formatting checks. It looks like these efforts have stagnated. Perhaps it would be possible to either a) restart work on lua-fmt or b) do something else? I was thinking perhaps we can just run a simple CI check for the style rules I've described above.

Mass-edits and ignore-revs

Once we've decided on a set of rules and implemented the controls to enforce them, another mass-formatting edit will be needed. We should add a .git-blame-ignore-revs file to filter this and previous such commits out.

TLDR

jlillis commented 3 years ago

I have a new rule to propose:

patrikjuvonen commented 3 years ago

For additional reference I had worked on a template for styles at https://github.com/multitheftauto/mtasa-resources/wiki/Lua-style-guide

jlillis commented 3 years ago

To get this moving again, can we decide on which line endings are the right line endings? I assume we'd want to use the same style as mtasa-blue, which is CRLF right?

patrikjuvonen commented 3 years ago

Specific style enforcement is required for JS and CSS.

jlillis commented 3 years ago

Specific style enforcement is required for JS and CSS.

Which settings would you like added to a repo editorconfig/prettierrc?

patrikjuvonen commented 3 years ago

Specific style enforcement is required for JS and CSS.

Which settings would you like added to a repo editorconfig/prettierrc?

For editorconfig, at least:

[*]
charset = utf-8
end_of_line = crlf
insert_final_newline = true
trim_trailing_whitespace = true

[*.{js,css}]
indent_size = 2
indent_style = space

For prettierrc, at least:

{
  "bracketSpacing": true,
  "singleQuote": true,
  "trailingComma": "all",
  "arrowParens": "always",
  "bracketSameLine": false
}

To get this moving again, can we decide on which line endings are the right line endings? I assume we'd want to use the same style as mtasa-blue, which is CRLF right?

Let's go with CRLF since no one has anything to say and it's not really that important. Can be changed later if needed. I use LF for all my projects elsewhere but in MTA we support only Windows anyway on client so CRLF is "traditional".

Perhaps eslint and stylelint could also be added.

jlillis commented 3 years ago

Another new rule in light of #300 - scripts must use client or source appropriately.

ArranTuna commented 2 years ago

I don't understand why you'd want 4 spaces to be used instead of tab when everyone uses tab and there's no benefit to it being 4 spaces? Tabs are specifically designed for indentation and why should everyone have to modify their notepad settings to make it 4 spaces especially if they don't want to use 4 spaces for other projects. Also I just tried converting a script file that uses tabs into 4 spaces and the file size increases 10%.

AlexTMjugador commented 2 years ago

I don't understand why you'd want 4 spaces to be used instead of tab when everyone uses tab and there's no benefit to it being 4 spaces?

Using tabulation instead of space characters is far from being universal, and there are arguments to favor either approach over the other. I personally prefer tabulation characters too, but if you search over the Internet there is no consensus on what to choose for every project.

What I think that matters most is sticking to a single standard, as mixing both tabs and spaces is arguably the worst approach. In addition to that, doing whatever is more natural, given the default configuration of the editors that the project developers tend to use and the conventions normally used in the broader Lua community, arguably reduces frictions further.

ArranTuna commented 2 years ago

Search results in mtasa-resources for: tab: mtasa-resources: 198,865 hits 4 spaces: 44,095 hits (divide by 4 = 11,023)

Tab is used to indent 94% of mtasa-resources therefore if there is to be a style rule it should be tab.

TheNormalnij commented 2 years ago

I've searched for "\n\t" (new line + tab) and "\n " (new line + space) Tabs: 105059 lines Spaces: 51619 lines It looks more realistic

jlillis commented 2 years ago

I don't understand why you'd want 4 spaces to be used instead of tab when everyone uses tab

I don't have a preference either way so long as some standard is enforced. In this case, I just copied what mtasa-blue does.

Fernando-A-Rocha commented 2 months ago

:shipit: Let's make a decision, establish the .editorconfig etc and do the mass style update commit?

jlillis commented 2 months ago

I think it might be good to hold off until the new style rules for mtasa-blue are put together so we can copy whatever is applicable.

T-MaxWiese-T commented 2 months ago

I think it might be good to hold off until the new style rules for mtasa-blue are put together so we can copy whatever is applicable.

I think it makes no sense to wait for it because the coding guidelines are mainly related to C++ code and the coding guidelines would be much less here. I also believe that it makes much more sense for C++ and the large scope. So for code in this repository I haven't really seen the need for coding guidelines yet. This is because Lua is a very simple, compact and undemanding scripting language and so are the MTA functions and events and the structure of the resources.

Fernando-A-Rocha commented 2 months ago

for code in this repository I haven't really seen the need for coding guidelines yet. This is because Lua is a very simple, compact and undemanding scripting language and so are the MTA functions and events and the structure of the resources.

There are a few code practices that should be enforced like using early-returns, adequate variable naming etc...

Fernando-A-Rocha commented 2 months ago

We could create and maintain the guidelines for contributing to this project here: https://wiki.multitheftauto.com/wiki/Default_resources_-_Contributing (WIP version)

Then, we can proceed with updating the repository with the new style rules, etc.

(#510)