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

add ReAssignmentFormattingRule and his tests #16846

Open
wants to merge 1 commit into
base: Pharo13
Choose a base branch
from

Conversation

AngelHely
Copy link
Contributor

This PR propose a rule for the indentation on assignments

Example:
good indentation

arg := 1

bad indentation

arg:=1
arg:= 1
arg :=1

@MarcusDenker
Copy link
Member

What I find problematic is that whitespace rules like this will lead to a lot of noise. We tried e.g. in the past rules related to whitespace and it is just so annoying that we had to disable them.

@AngelHely
Copy link
Contributor Author

Maybe we can manage to disable formatting rules by default. Otherwise, we can forget formatting rules.

@astares
Copy link
Member

astares commented Jul 1, 2024

so annoying that we had to disable them

Depends on the process and right tools. We need to display ugly and bad code and stick programmers nose to it. Punish the ones who make the house dirty and help and support the one who keep it clean.

It must be annoying to the one who violates the rules - but yes: also lower the pain for the final integrators. The primary enemy is just LAZINESS so beside the rules we need more predefined "autofix" implementations together with the rules. The mechanisms are already there and then there should be no excuse.

If we say "sometimes you must have clean code" and "somtimes it is fine to be ugly" we make it worse - essentially nobody sticks to rules and rules are useless. Code will be ugly, bugs creep in and nobody cares.

IMHO it must already lament on commit and when doing the PR. Violations are not acceptable - simple rule(s) that everybody understands and then follows.

If others are able to work that way why should it not be possible for Pharo?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants