Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Dont call mutators as methods #5647

Merged
merged 4 commits into from
Sep 2, 2024
Merged

Dont call mutators as methods #5647

merged 4 commits into from
Sep 2, 2024

Conversation

pxpm
Copy link
Contributor

@pxpm pxpm commented Sep 2, 2024

WHY

BEFORE - What was wrong? What was happening before this PR?

Not previously tested, broken 🤷‍♂️

We had a use-case outside of our test suite that was reported in #5645

When developer added an Attribute (an accessor/mutator) as the last parameter of a relation string, we would wrongfully interpret it as a relationship.

AFTER - What is happening after this PR?

We properly infer that's not a relationship. Added tests for that scenario too, nothing else is broken, we should be fine 👍

HOW

How did you achieve that, in technical terms?

Wrote the test, expect it to fail, it failed, fixed the relevant code, all green 🟢

Is it a breaking change?

No

How can we test the before & after?

As @karandatwani92 pointed out in #5645 (comment)

@pxpm pxpm merged commit 1eaebf7 into main Sep 2, 2024
3 checks passed
@pxpm pxpm deleted the dont-call-mutators-as-methods branch September 2, 2024 13:49
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants