mehcode / config-rs

⚙️ Layered configuration system for Rust applications (with strong support for 12-factor applications).
Apache License 2.0
2.43k stars 206 forks source link

Fix nested arrays (by reworking array handling) - backport #486

Closed ijackson closed 7 months ago

ijackson commented 8 months ago

Backport of #465 to 0.13.x branch. Fixes #464 there.

matthiasbeyer commented 8 months ago

Please re-do the commits with the -x flag on git-cherry-pick (as noted here) so that the backported commits refer to the original ones.

ijackson commented 8 months ago

Please re-do the commits with the -x flag on git-cherry-pick (as noted here) so that the backported commits refer to the original ones.

Ah sorry, didn't see that comment. FTR, I did:

for f in `git-rev-list --reverse --topo-order nested-array~4..nested-array`; do echo git cherry-pick -x $f; done | sh -xe
matthiasbeyer commented 8 months ago
for f in `git-rev-list --reverse --topo-order nested-array~4..nested-array`; do echo git cherry-pick -x $f; done | sh -xe

The following would have been sufficient:

git cherry-pick -sx $(git merge-base master^1 master^2)..master^2
Explanation of the above command Explanation: The above means cherry-pick the changes: * from the merge base of the commit to the left of current master and the commit to the right of current master (the last commit of the previously merged PR) * to the commit to the right of current master (the last commit of the previously merged PR) You can replace the `git cherry-pick -sx` with `git log --oneline` and you'll see that this is exactly the range you want to pick.

But no worries! Log looks good now, let's see what CI tells us!


It seems that we have to bump the MSRV for the next 0.13.x release ... I don't like that but I don't see a better way, tbh. I think I'll do that and rebase this PR then. No need to worry for you!

ijackson commented 8 months ago

I think we could fix this on the 0.13 branch by committing a suitable Cargo.lock. Would you like me to do that?

ijackson commented 8 months ago

It seems that we have to bump the MSRV for the next 0.13.x release ... I don't like that but I don't see a better way, tbh. I think I'll do that and rebase this PR then. No need to worry for you!

Please let me have a go at fixing it with a Cargo.lock instead? I will try that tomorrow; it's getting rather late for me here now.

matthiasbeyer commented 8 months ago

Please let me have a go at fixing it with a Cargo.lock instead? I will try that tomorrow; it's getting rather late for me here now.

Yes, feel free to try. Although I am not sure about whether that would help, as downstream might still run into the issue that they have a new log crate version (for example) but an compiler too old for that crate version.

But I see where you're going. I'll stop my efforts for the MSRV update on the release branch (#487) now!

Have a good night! :owl:

ijackson commented 8 months ago

I have succeeded, by providing a Cargo.lock. I will tidy up my branch and make an MR of it.

ijackson commented 7 months ago

I've rebased this to pick up the CI fix from #496

matthiasbeyer commented 7 months ago

I am on vacation until late tomorrow. I hope I can get back to this as soon as possible.