Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable automated source code formatting #3142

Open
schlomo opened this issue Jan 29, 2024 · 13 comments
Open

Enable automated source code formatting #3142

schlomo opened this issue Jan 29, 2024 · 13 comments
Assignees
Labels
discuss / RFC ReaR Project The ReaR project itself and the tooling we use to work on it. waiting for info

Comments

@schlomo
Copy link
Member

schlomo commented Jan 29, 2024

See also #3138 (comment) for context.

I'd like to be able to use automated source code formatting with ReaR, and the go-to-tool for that seems to be shfmt which is also supported in our .editorconfig file and in various IDEs (I'm currently using Visual Studio Code with https://marketplace.visualstudio.com/items?itemName=foxundermoon.shell-format).

Our https://github.com/rear/rear/wiki/Coding-Style has some rules that conflict with the abilities of this formatter, specifically adding extra white space or padding around code fragments or before ;.

I wasn't able to find a shell script formatting solution that works in the IDEs that allows such a coding style, and I believe that automated consistent formatting is more important. Therefore I'd like to suggest to adjust our coding style to also allow (or demand?) a more compact way of writing Bash and to recommend using a shell script formatter to format the code.

I'm happy to submit the PR with the change that reformats all lines, although that would kill git blame and therefore I'd rather prefer that we reformat files as we work on them.

Personally I'd prefer to not require a "clean" git blame as a requirement over consistent and automated code formatting.

What do you think @rear/contributors?

@schlomo schlomo self-assigned this Jan 29, 2024
@schlomo schlomo changed the title Change coding style to allow working with automated shell script formatting Enable automated source code formatting Jan 29, 2024
@jsmeix
Copy link
Member

jsmeix commented Feb 9, 2024

Personally I don't worry about coding syntax style differences.

For example personally I don't care if

if CONDITION ; then
    ...
    ...
fi

or

if CONDITION
then ...
     ...
fi

is used because both are reasonably easy
to read and - most important - to understand.

Therefore
https://github.com/rear/rear/wiki/Coding-Style
talks initially about "hints" - not "rules":

Here is a collection of coding hints
that should help to get a more consistent code base.

Don't be afraid to contribute to Relax-and-Recover
even if your contribution does not fully match all this coding hints. 
...
Nevertheless try to understand the idea behind this coding hints
so that you know how to break them properly
(i.e. "learn the rules so you know how to break them properly").

The overall idea behind this coding hints is:

Make yourself understood

So as long as it is reasonably easy for me
to understand some code I do not care about
coding syntax style.

For example I prefer spaces as separators
basically everywhere in my code for example as in

if for x in this that ; do echo $x ; done ; then
    echo OK
fi

because personally I find that easier to read
than

if for x in this that; do echo $x; done; then
    echo OK
fi

But this is really not something which makes
the code needlessly hard to read
so I don't worry about such coding syntax style details.

In contrast for example I think that backticks
make code needlessly hard to read - in particular
when also single quotes are used things look confusing.

@jsmeix
Copy link
Member

jsmeix commented Feb 9, 2024

@schlomo
do you perhaps propose
to run an automated source code formatter
over all existing ReaR code?

Copy link

Stale issue message

@jsmeix
Copy link
Member

jsmeix commented Apr 10, 2024

@schlomo
Copy link
Member Author

schlomo commented Apr 10, 2024

@jsmeix the problem we saw just now is actually only a problem of the transition: If all the source code would have been formatted by a tool then whenever somebody works on code the formatting tool ensures that there are no needles formatting changes.

As we have not yet reformatted the ReaR code via tool we keep seeing this problem.

Therefore indeed I propose to decide upon a tool and "convert" the source code with a single PR to that automated formatting.

The main downside I can see to this is that we loose the easy git blame history because then the author of that PR will be visible in all changed lines.

The alternative is IMHO to give up on very consistent ReaR source code formatting and accept PRs that mostly look good without too much attention to the details.

@jsmeix
Copy link
Member

jsmeix commented Apr 10, 2024

My personal preference is to give up on
consistent ReaR source code syntax style and
accept PRs that properly implement actual functionality
without much attention to coding style details
provided the coding style looks acceptable ("cum grano salis")
which means as long as the code is easy to understand
i.e. as long as the code is maintainable which means
that at any time later others still understand the code
so they can properly fix and enhance the code as needed.

Cf. my above
#3142 (comment)

@jsmeix jsmeix added discuss / RFC ReaR Project The ReaR project itself and the tooling we use to work on it. labels Apr 10, 2024
@jsmeix
Copy link
Member

jsmeix commented Apr 10, 2024

@gdha @pcahyna
please comment here what your personal opinion is
regarding automated source code formatting and
regarding a consistent ReaR source code syntax style.

@schlomo
Copy link
Member Author

schlomo commented Apr 10, 2024

@jsmeix I can also live with accepting different coding styles and formattings, but then I'd expect it to be OK to change the formatting as part of working on code and not be enslaved to the "original" formatting

@jsmeix
Copy link
Member

jsmeix commented Apr 10, 2024

@rear/contributors
when a majority of us active ReaR upstream maintainers
prefer a consistent ReaR source code syntax style
it would be perfectly OK for me to enforce that by
"converting" our whole source code with a single PR
to an automated formatting
because
I do not care much what source code syntax style is used,
cf. my above
#3142 (comment)

But if we "convert" our whole source code with a single PR
to an automated formatting then those who want that must
carefully review this PR to avoid that the conversion may
mess up things where "exceptional" syntax stlye is needed.
For example I am thinking about things
like strings over several lines

    my_text="This text
is several
lines long"

where automated indentation would change the string value.

@jsmeix
Copy link
Member

jsmeix commented Apr 10, 2024

@schlomo
yes of course, when you work on some code
(in particular when you overhaul some code),
you become owner of the the new code version
(or at least the one who is to "blame" in git)
and as owner (blamable/culprit) it is your code
where only you decide what syntay style you like
(provided others can understand your code).

I did this very often when I completely overhauled a script
that I used my personally preferred coding style syntax
everywhere in my (then it became 'my') overhauled script.

@jsmeix
Copy link
Member

jsmeix commented Apr 10, 2024

@rear/contributors
when we agreed here how we like to deal
with source code syntax style, I will overhaul
https://github.com/rear/rear/wiki/Coding-Style
appropriately.

@gdha
Copy link
Member

gdha commented Apr 10, 2024

@jsmeix @schlomo I'm okay with the Coding-Style of ReaR.

@jsmeix
Copy link
Member

jsmeix commented Apr 10, 2024

Perhaps a good argument against a
consistent ReaR source code syntax style
is that this would cause needless problems
with new contributors who use their own coding style.

I would like to avoid unhelpful discussions about
their own coding style (provided we understand their code)
and focus on whether or not they did properly implement
their intended added/enhanced/changed functionality.

I think that coding style discussions could become
rather quickly very unhelpful because coding style
is a very personal thing so trying to teach others about
their coding style could be soon perceived as intrusive.

I even think that coexisting different coding styles in ReaR
are a good sign of a healty/open/free culture in our project.
In particular when on the other hand we are strict that
we only accept contributions where we clearly understand
what a contribution actually contributes, cf.
#3202

Simply put:
When we tell our contributors that their own coding style
does not matter for us but it does matter for us that
we clearly understand what they actually contribute,
such a condition should be willingly accepted
and even appreciated by our contributors.

schlomo referenced this issue Apr 10, 2024
…der.sh to layout/save/default/445_guess_bootloader.sh (issue1266) and some other minor adaptions that are related to issue1266
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discuss / RFC ReaR Project The ReaR project itself and the tooling we use to work on it. waiting for info
Projects
None yet
Development

No branches or pull requests

4 participants