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

Refactor parseShapes function to simplify property checks and improve readability #1709

Closed

Conversation

Ayoub-Mabrouk
Copy link

Refactor parseShapes function to simplify property checks and improve readability

  • Combined checks for properties.P and properties.L into a single conditional statement.
  • Replaced separate blocks for properties.P and properties.L with a ternary operator to set the type (0x01 for P, 0x02 for L).
  • Updated the loop to start from 0 instead of 1 for consistency and clarity.
  • Preserved the logic for reading buffer values and updating the buffer position after each iteration.

These changes streamline the function without altering its core behavior.

@dhensby
Copy link
Collaborator

dhensby commented Nov 14, 2024

I'm not a huge fan of the LLM contribution, but this seems an acceptable change.

Please amend the commit message as per conventional commit standard

@Ayoub-Mabrouk
Copy link
Author

I'm not a huge fan of the LLM contribution, but this seems an acceptable change.

Please amend the commit message as per conventional commit standard

thanks for reviewing @dhensby, i try my best to make the pull request as clean as possible that's why i used it.
the commit message is clear now

@dhensby
Copy link
Collaborator

dhensby commented Nov 15, 2024

The commit must be amended, please. This is now 3 commits and not 1

…ed properties.P and properties.L checks with a ternary operator for type assignment (0x01 for P, 0x02 for L). Updated loop start for clarity, preserving buffer handling and core behavior.
@dhensby
Copy link
Collaborator

dhensby commented Nov 18, 2024

Thanks - after due consideration, I'm afraid I won't be accepting this (or your other PRs) because I believe they have been generated my a LLM.

Whilst this repo doesn't specifically have a policy on LLM based contributions, I don't think it's appropriate to allow contributions that have not been authored by a human. Not necessarily because these contributions are bad, but if I start accepting LLM based contributions it will start to encourage low-effort clout chasing in the repositories that I maintain. It will also save having my limited mental capacity exhausted by the practically infinite capacity of LLMs.

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