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

fix: convert the keys in the array to lowercase #608

Closed
wants to merge 2 commits into from

Conversation

rise0chen
Copy link

@rise0chen rise0chen commented Dec 5, 2024

fix #609

turn keys to lowercase to enable cross overrides not convert the keys in the array to lowercase. This PR fix it.

Before fix:
image

After fix:
image

@epage
Copy link
Contributor

epage commented Dec 6, 2024

Is there an issue that this should be linking back to? We recommend any non-trivial (like beyond a typo) PRs to first start as issues so we make sure we have a mutual understanding the problem and the right direction to go in solving it.

When posting PRs, we also recommend the PR having at least two commits

  1. A test showing the current behavior
  2. The fix with the update to the test so it pass

The diff for the test makes it easy to verify tests are testing the right thing and make it very clear what the intended change is.

@rise0chen
Copy link
Author

Before fix:
image

image

@rise0chen
Copy link
Author

I have split it into two commits.

I haven't submit an issue, should I resubmit an issue?

@epage
Copy link
Contributor

epage commented Dec 9, 2024

I haven't submit an issue, should I resubmit an issue?

Yes. PRs are not great places for discussing problems and potential solutions. They are much more likely to come and go and a lot of that background gets lost or bifurcated.

@rise0chen
Copy link
Author

fix #609

@epage
Copy link
Contributor

epage commented Dec 13, 2024

Going to close for now as we should consider other related issues like #531 in fixing issues introduced by #354.

@epage epage closed this Dec 13, 2024
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.

can't get item in array at v0.14
2 participants