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

Removed the if-else and replaced with clearer data flow. #1311

Merged
merged 1 commit into from
Nov 13, 2024

Conversation

mosbat
Copy link
Contributor

@mosbat mosbat commented Nov 4, 2024

Just minor change in coding style to make code clearer and easier to manage.

I didn't create a Jira issue since as per the documentation, it's a minor issue and not a bug or feature. I'm getting accustomed to this project.

P.S.
This is my first PR with this project, I did go through your documentations/guides; I'd appreciate a bit of patience in case I missed something or if I didn't follow a certain point instead of rejecting my PR outright and leaving me confused. Thank you

@garydgregory
Copy link
Member

garydgregory commented Nov 4, 2024

@mosbat
Why did you remove the comment? This change feels pretty arbitrary anyway.

@mosbat
Copy link
Contributor Author

mosbat commented Nov 4, 2024

@mosbat Why did you remove the comment? This change feels pretty arbitrary anyway.

I thought at the beginning that it should be obvious without the comment; but thinking about it again, I put back the comment just now since the comment makes it clearer.

It's a minor change since I hate if-else statements :) since they are nighmarish but it can be also just personal preference :)

@mosbat
Copy link
Contributor Author

mosbat commented Nov 13, 2024

@garydgregory I know how resolving conflicts can be painful :D

I was resolving the conflict and got a bit confused when I saw that the only change was the comment lol thought I messed up the conflict resolution haha

@garydgregory garydgregory merged commit 267242e into apache:master Nov 13, 2024
16 of 18 checks passed
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