a PR claiming a fix must validate it

[dimpase]

#38163 claims to fix #38159, but does not provide a test showing this.

Thus, if #38163 is closed, it will trigger closing #38159. Thus the claim should be removed, or a followup PR or issue opened.

[dimpase]

Another problem with #38163 is that it has made an optional package tdlib dependent upon an experimental package sagemath_environment. This is not how it is supposed to work, an optional package can only depend on optional and standard packages. @dcoudert - please retract your positive review.

@vbraun - please refrain from merging #38163 in its present form.

EDIT - sorry, I take this back. #38163 is OK in this sense - the breaking change was added much earlier, in #35661

[dimpase]

And the blocker #38190 also points at #38163 being responsible for breaking the build in some cases.

[saraedum]

Another problem with #38163 is that it has made an optional package tdlib dependent upon an experimental package sagemath_environment. This is not how it is supposed to work, an optional package can only depend on optional and standard packages.

Could you explain where that dependency is? It's not in dependencies at least.

[dimpase]

with #38163, it is in the build dependencies:

cat build/pkgs/sagemath_tdlib/dependencies
tdlib cysignals boost_cropped | $(PYTHON_TOOLCHAIN) sage_setup sagemath_environment cython pkgconfig $(PYTHON)

[saraedum]

I don't see that in the changeset of the PR. What am I doing wrong?

[dimpase]

perhaps it was already there, before this PR (afk now). Do a git blame after the checkout?

[dimpase]

then this PR by itself is innocent (apologies as I didn't check this), but the blocker still stands.

[dimpase]

This can be closed as soon as there is a test certifying that #38159 got fixed.

[S17A05]

Since #38163 does not claim to fix #38159 anymore and @dimpase set the PR back to "positive review", I am closing this issue - please reopen if I misunderstood.

[dimpase]

@S17A05 :

#38163 claims to fix #38159, but does not provide a test showing this.

Thus, if #38163 is closed, it will trigger closing #38159. Thus the claim should be removed, or a followup PR or issue opened.

Is there a followup?

[S17A05]

@S17A05 :

#38163 claims to fix #38159, but does not provide a test showing this.

Thus, if #38163 is closed, it will trigger closing #38159. Thus the claim should be removed, or a followup PR or issue opened.

Is there a followup?

Not that I know of; however, the claim was removed - is that not what you were asking for (alternatively to a followup)? After all, issue #38159 will stay open independently.

[dimpase]

closing in favour of #38159

[dimpase]

@S17A05 - sorry for not realising that #38159 is perfectly good to track this further. mea culpa.