Page MenuHomePhabricator

config: test priority involving alias
ClosedPublic

Authored by marmoute on Jan 29 2021, 8:39 PM.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

marmoute created this revision.Jan 29 2021, 8:39 PM
pulkit added a subscriber: pulkit.Jan 31 2021, 12:51 PM
pulkit added inline comments.
tests/test-config.t
448

From higher level do you mean the file which is read later or some kind of levels in configs?

marmoute added inline comments.Jan 31 2021, 2:13 PM
tests/test-config.t
448

The file which is read the latest (actually, the value which is read the latest because include make this non-trivial).

pulkit added inline comments.Jan 31 2021, 2:49 PM
tests/test-config.t
448

Can you rephrase the comment then? I confused it with config aliases.

marmoute added inline comments.Feb 1 2021, 12:26 PM
tests/test-config.t
448

I don't know what confuse you, so I am not sure what kind of rewriting we need.

durin42 requested changes to this revision.Feb 2 2021, 11:50 AM
durin42 added a subscriber: durin42.
durin42 added inline comments.
tests/test-config.t
448

I still have no idea what "higher level" means: is the highest level the systemwide config or is that lowest level? Perhaps "most-global" or "repo-local" instead of "higher level".

This revision now requires changes to proceed.Feb 2 2021, 11:50 AM
baymax updated this revision to Diff 25513.Feb 9 2021, 6:29 PM

โœ… refresh by Heptapod after a successful CI run (๐Ÿ™ ๐Ÿ’š)

durin42 requested changes to this revision.Feb 11 2021, 11:25 AM

This still requires changes to resolve my comments.

(Perhaps baymax could be altered to not refresh patches that are in "needs changes" state, since it clears that bit.)

This revision now requires changes to proceed.Feb 11 2021, 11:25 AM
marmoute requested review of this revision.Feb 11 2021, 11:47 AM

This still requires changes to resolve my comments.

The series has received multiple comment update that should clarify your request. Can you recheck the series (and clarify your issue if you still have some?)

(Perhaps baymax could be altered to not refresh patches that are in "needs changes" state, since it clears that

In this case, the new version is expected to cover the requested change, so baymax behavior was right there.

I'm still confused here.

tests/test-config.t
451

I'm still confused by the "highest level (read later)" here - does this mean the command-templates version is higher, or the ui one is higher?

marmoute added inline comments.Feb 11 2021, 12:10 PM
tests/test-config.t
451

It depends on which one is the last be read. this (should) behave the same as if the file were setting the same value. Except each file set value for different, yet synonyms, config option. (cf initial report of the issue by Yuya a while back).

baymax updated this revision to Diff 25570.Feb 11 2021, 1:46 PM

โœ… refresh by Heptapod after a successful CI run (๐Ÿ™ ๐Ÿ’š)

pulkit accepted this revision.Feb 27 2021, 12:28 PM

Thanks for adding detailed comments.

This revision was not accepted when it landed; it landed in state Needs Review.
This revision was automatically updated to reflect the committed changes.