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

Apply code formatter to classes of module 'udig.tools.feature.util' #540

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

sschulz92
Copy link
Contributor

No description provided.

@fgdrf
Copy link
Contributor

fgdrf commented Oct 8, 2021

@sschulz92 thanks for providing this pull-request. I totally understand thats easier to maintain the code if you don't have formating clitches which causes merge issues and even more bad-readable code.

I'd prefer if we leave the code how it is, and change its only if required, e.g. for new features or bugfix-changes. Would you agree.

IMHO, pros for changing only on occasion:

Downside to keep the code as it is:

  • codebase is kind of inconsitent regarding coding-rules (for a longer time) and might confuse others to getting started

Contra for boy-scout rule: its not trivial to see the real change because formatings just hide the fix. IMHO it makes it harder to merge (and backport bugfixes to older releases - which is not really a use-case for uDig :P)

Therefore we updated the Getting started guide recently with #501 and I'd suggest to change classes/packages/modules only on occassion.

Opinions?

@sschulz92
Copy link
Contributor Author

With most of my pull-requests I want to improve the quality of the repository in general. This can be done by fixing trivial warnings such as missing NLS hints.

One of my temp. goals is to reduce the amount of warnings shown in Eclipse. I might continue changing the use of SubProgressMonitor with SubMonitor - including the formatting part. Do you agree on that?

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.

2 participants