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

Data flow style change from if-else to water flow. #16118

Conversation

mosbat
Copy link

@mosbat mosbat commented Nov 18, 2024

Hi guys, I'd like to contribute even with small changes or improvements for practice or just to purely satisfy my coding habits.

I found an if else statement and after studying/observing it for a while, it felt a bit awkward (understandably); and would love to propose just a mere style change from if-else to waterfall style.

I do not expect you to like my changes; but if you'd like to discuss them or you have the same passion as I do, I'm happy to listen to comments and maybe hints.

I was thinking btw, the class is deprecated but still seems to be in use, or am I wrong?

@mosbat
Copy link
Author

mosbat commented Nov 18, 2024

fixes #16119

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Nov 18, 2024
@sjohnr sjohnr self-assigned this Nov 18, 2024
@sjohnr
Copy link
Member

sjohnr commented Nov 18, 2024

@mosbat Thank you for wanting to contribute!

I'd like to contribute even with small changes or improvements for practice or just to purely satisfy my coding habits.

Changing code style to suit preference is something best done in your own projects or fork of Spring Security. I don't find precedent for accepting PRs that change code style simply due to preference. Please identify an actual issue with the code, and please consider opening an issue first to discuss it as outlined in the contributing guidelines (unless it is a trivial change that contains an obvious fix, such as a typo in documentation).

I do not expect you to like my changes; but if you'd like to discuss them or you have the same passion as I do, I'm happy to listen to comments and maybe hints.

This issue tracker is not a discussion forum, so I don't feel it would be productive to have these discussions here unless it is directly relevant to an issue.

For the above reasons, I am going to close this PR. Please consider looking at the ideal-for-contribution label as an alternative to try your hand at small issues/PRs.

@sjohnr sjohnr closed this Nov 18, 2024
@sjohnr sjohnr added for: stackoverflow A question that's better suited to stackoverflow.com and removed status: waiting-for-triage An issue we've not yet triaged labels Nov 18, 2024
@sjohnr
Copy link
Member

sjohnr commented Nov 18, 2024

Apologies I missed the issue you opened first. However, I will also close the issue as a duplicate.

@sjohnr sjohnr added type: enhancement A general enhancement status: declined A suggestion or change that we don't feel we should currently apply and removed for: stackoverflow A question that's better suited to stackoverflow.com labels Nov 18, 2024
@mosbat
Copy link
Author

mosbat commented Nov 19, 2024

Apologies I missed the issue you opened first. However, I will also close the issue as a duplicate.

Thank you for the feedback. I understand that code style consistency is important, but I believe readability is also a crucial factor in maintainability, and I was hoping to get your thoughts on that aspect. I'll take another look at the project and see if I can identify any other issues that might be more in line with the project’s current goals.

I'm still familiarizing myself with the guidelines and process here, and I appreciate your time and guidance

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: declined A suggestion or change that we don't feel we should currently apply type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants