Update Spinner API, fix #11004

[embray]

Reverts #11771

Re-merge #11046 once we find out what in the name of ... would possibly cause this to cause the RTD builds to hang.

EDIT: Fix #11004

[pllim]

Forget about reredirect, let's rerevert 😹

[pllim]

Well, no need to worry about the change log check failure right now...

[pllim]

If my hunch is correct, changing ProgressBarOrSpinner to just ProgressBar in utils/data.py will fix RTD, though even if that is the case, it is not the correct fix.

[bsipocz]

Please do not open branches in the main repo, but rather in your fork.

[pllim]

I think this is a side effect from pressing the "revert" button on GitHub.

[bsipocz]

Yes, I suppose it's down to GitHub. Yet, in that case stuff like this needs to be merged asap and the branch deleted, or they will show up in forks, too.

You can argue is not a big deal, but as a rule of thumb we should do what we preach about workflow.

[pllim]

Yes, understood. If I get lucky over at #11779 , I might move the fix over there and close this, though I am unable to promise a fast turnaround. Sorry for any inconvenience caused!

[embray]

I don't really see the problem. I guess in the case of the GitHub "Revert" button it's an exception to the normal rule. It's too bad it doesn't give an option to make the branch in a fork, like the online file editor does. But it's not "preaching" anything.

[embray]

Can confirm that the way Spinner.update() behaves makes no sense, now that I look at this again. If you pass it a large value it will just sit there spinning forever.

[embray]

Should we keep the existing towncrier file or move it to the new PR number?

[pllim]

Oh, good! I am glad you fixed it. I was dabbling on the doc side and was sliding into a rabbit hole.

Should we keep the existing towncrier file or move it to the new PR number?

I dunno. I will never grok this towncrier stuff. @saimn or @Cadair , what is your advise here?

[pllim]

p.s. Since the original PR was flawed anyway, maybe just change the PR number to match this one?

[pllim]

Since the CI passed, I am approving, with the expectation that y'all will sort out the change log stuff. Thanks!

[saimn]

I'm a bit lost by the revert of revert, but it seems this PR is the one fixing the bug so I guess the PR number should be updated in the fragment filename ?

[bsipocz]

I don't really see the problem. I guess in the case of the GitHub "Revert" button it's an exception to the normal rule. It's too bad it doesn't give an option to make the branch in a fork, like the online file editor does. But it's not "preaching" anything.

But this is not a simple revert, but a revert of a revert with some additional changes. So it would have been nicer to make readd that PR along with the fixes in a separate feature PR. (I also have strong opinions of any reverts should be done from a feature branch rather than in the main repo, after all the revert commit can be added locally. I know it's an easy feature of github, but we also doesn't use other features it offers as they don't fit into the workflow, and the revert should be part of that collection).

[embray]

Yeah, I think I made a mess by making a revert of a revert. Should have just introduced a new PR.

Perhaps we should add something in the developer docs against using GitHub's built-in revert feature?

[embray]

I'll change the PR number for the changelog entry.

[pllim]

Re: developer docs -- I am neutral on this. We don't revert very often.

[pllim]

Thanks!