Skip to content

Fix functions that take a field path #488

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

Merged
merged 6 commits into from
Jan 24, 2023
Merged

Conversation

rhansen
Copy link
Collaborator

@rhansen rhansen commented Jan 21, 2023

  • Fix deepGet to automatically dereference pointers when walking the field path.
  • Support accessing map entries with an empty string key.
  • Extend field paths to support slice/array element access.

@buchdag
Copy link
Member

buchdag commented Jan 23, 2023

Interesting, do you have something specific in mind made possible by those improvements ?

buchdag
buchdag previously approved these changes Jan 23, 2023
@rhansen
Copy link
Collaborator Author

rhansen commented Jan 23, 2023

Yes, I will need it for my proposed fix for nginx-proxy/nginx-proxy#1504.

@rhansen
Copy link
Collaborator Author

rhansen commented Jan 24, 2023

Doh, I didn't realizing that a simple rebase was enough to dismiss a review. And resetting it back doesn't restore the review. 🙁

@rhansen rhansen requested a review from buchdag January 24, 2023 01:42
This will make it easier to fix bugs and add new features.
This makes it possible to use the empty string as a map key.
This matches the behavior of Go, and makes it possible to use `groupBy` and
friends on a slice of `*RuntimeContainers`.
@rhansen rhansen force-pushed the deepGet branch 3 times, most recently from 552a9dc to 298e650 Compare January 24, 2023 03:03
@buchdag
Copy link
Member

buchdag commented Jan 24, 2023

Doh, I didn't realizing that a simple rebase was enough to dismiss a review. And resetting it back doesn't restore the review. 🙁

Yeah, since a rebase changes the commits hashes, GitHub view them as different commits and marks the previous review(s) as stale. Out of curiosity, did it automatically re-request a review from me or did you have to do it manually ?

@rhansen rhansen merged commit 616392c into nginx-proxy:main Jan 24, 2023
@rhansen rhansen deleted the deepGet branch January 24, 2023 15:42
@rhansen
Copy link
Collaborator Author

rhansen commented Jan 24, 2023

Yeah, since a rebase changes the commits hashes, GitHub view them as different commits and marks the previous review(s) as stale.

I was hoping it was smart enough to realize the commits were a rebase (that each commit's tree hash was the same as if I had chosen "rebase and merge"}, or, if not, that I could undo the rebase. Sadly no on both counts.

Out of curiosity, did it automatically re-request a review from me or did you have to do it manually ?

I re-requested the review manually.

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