Repository navigation
Conversation
|
|
|
You're right, and my description was wrong. Thanks for digging out the actual history. I checked 5ca8592 and the loop was there: for lang in implementations:
param = f'with_{lang}'
if with_all:
args[param] = with_all
enabled_implementations += args[param]and 9a21925 dropped it along with On whether the feature is needed, one thing that I think explains the silence: nothing in the repository passes So the people who would hit this are contributors following that page locally, and the failure is a Either direction is fine by me and it's your call — I'd just rather the docs and the code agreed:
If you do keep it, two differences from the pre-22.0.0 behaviour worth deciding on, since neither is forced:
|
|
Let's remove this feature. If someone needs this again, let's revisit this. |
--with-all stopped working in 22.0.0: apacheGH-47650 (9a21925) removed the loop in integration() that implemented it when select_testers() took over, so the flag has been accepted and ignored since then. Nothing in the repository passes it. CI goes through ARCHERY_INTEGRATION_WITH_*, and verify-release-candidate.sh and generate_files_for_endian_test.sh pass explicit --with-<lang> flags, so remove the option rather than restore it. docs/source/format/Integration.rst was the one place still telling contributors to use it, so rewrite that example to enable each implementation explicitly, matching the two examples above it.
3d43198 to
e6299eb
Compare
|
Done — switched this over to removing No test, since Click rejects an unknown option on its own and a test asserting a flag's absence would only have to be deleted if it ever came back. I did check that One aside, not part of this: |
Rationale for this change
archery integration --with-allhas not worked since 22.0.0. It was implemented by a loop inintegration(), which GH-47650 (9a21925) removed along withenabled_implementationswhenselect_testers()took over, so the flag has been accepted and ignored ever since.This PR originally restored it. On review the preference was to drop it instead: nothing in the repository passes
--with-all— CI goes throughARCHERY_INTEGRATION_WITH_*,verify-release-candidate.shandgenerate_files_for_endian_test.shpass explicit--with-<lang>flags — and no bug reports have come in. If it turns out to be wanted, it can come back.The one option that doesn't work is leaving things as they are, because
docs/source/format/Integration.rststill told contributors to run it:Closes #55161.
What changes are included in this PR?
--with-alloption and its parameter fromarchery integration.Integration.rstto enable each implementation explicitly, in the same style as the two examples above it.Nothing else in the tree referenced
with_all.Are these changes tested?
No new tests. Click rejects an option that no longer exists, so there is no behaviour of our own left to pin down, and a test asserting the absence of a flag would just have to be deleted if it ever came back.
What I checked locally:
archery integration --with-all --run-ipcnow exits 2 withNo such option '--with-all', and Click suggests the real--with-<lang>options.--with-cpp=1still enables only C++.select_testers()and inspecting what it receives.archerytest run: 26 failed / 24 passed / 1 skipped, identical to pristinemain. The 26 are pre-existingdockerFileNotFoundErrors on this machine, nothing to do with this change.flake8 --config python/setup.cfgclean.Are there any user-facing changes?
archery integration --with-allis no longer accepted and now errors instead of being silently ignored. Since it has done nothing since 22.0.0, no working invocation changes behaviour.Was AI used for this PR?
In accordance to the AI generation guidelines, please disclose below whether and how AI was used in this PR.
Found with a static scan for parameters a function accepts and never reads. The code and this description were AI-assisted. My first description of the history was wrong — it claimed the flag had never worked — and that was corrected in review; the account above was read from the commits.
PR code and description written by:
Reviewed before submission by: