Notifications: logic behind non-latest version

[humitos]

It seems there is some hidden logic in the old implementation of this feature. This comment called my attention regarding branches vs. tags: fabric/fabric#2255 (comment). There is a link to readthedocs/readthedocs.org#3268 which explains a little bit of the history here.

We should clearly define what's the logic behind our semver comparisons.

[humitos]

At this point, I think we should implement "the most basic and strict version of semver" first and then allow people to write their javascript function to replace this basic logic. We will never cover all the cases and, in fact, I found that a lot of cases are edge cases:

• calver
• tags have priority over branches
• versions with prefixes like release/2.0 or v3.1, etc

I think this will give users something that works in a reliable way for specific use case, but also the flexibility to customize it as they want if they have a particular use case.

[humitos]

The logic I have in mind would be similar to:

The simple case, is the ✅ valid version: (semver.valid). They look like 1.2.1 and v3.1.6 and =4.1.1. However, the ❗ invalid version is more complex and we need to loose it a little here:

• it has to have / on its name:
  • release/2.1
  • rel/2.1
  • development/release/4.1.1
• it has to have at least a "minor" part on the name (release-1 is not valid)
• the "major" part of the version has to be isolated (between / or - or .)
  • release-candidate-2.1 ➡️ 2.1.0
  • project-name-1.6/release ➡️ 1.6.0
  • test-404-page ➡️ invalid (otherwise it gives us 404.0.0 when calling semver.coerce)

While writing this down, I found it's not super accurate nor easy to explain, but I think this implementation could cover most of the use cases. We can tweak it a little as we go.

However, people with strict/different needs (like calver), they should write their own logic for this.

As examples, our test-builds project detects the highest version as custom-404-page because when coerced it's 404.0.0. By disabling that version, it detects py3.10 as the highest version when coerced as 3.10. I'm not sure how to solve these cases 🤔

[humitos]

@stsewd @agjohnson @ericholscher do you have a better and simpler idea for this?

[agjohnson]

Hrm, honestly I'm not sure. Comparing versions that are already semver makes sense, but coercion looks like the wrong tool to rely on here. For every case we get right using coercion, I'm sure there will be plenty others that we get wrong.

I'd say if it isn't obviously a semver version, we can't support it with this warning. At least not right now. I still think there is more was could do to give the user the ability to tell us what is latest, instead of us using some amount of magic to guess. We've talked about surfacing this sort of behavior in automation rules, perhaps related to latest/stable rules, which I still think is a really good idea.

Actually, thinking more, I sort of just want a Version.is_outdated that the user controls manually or through automation rules.

[humitos]

What about a pretty simple logic to start with?

• if there are the latest and stable versions active, we show "you are reading the development version, the stable version is at ..." when reading latest
• if there are stable and more active versions, when navigating other versions we show "you are not reading the stable version, read it..." if the version people is reading is not the stable one

I think this simple approach can work to start with. In the end, the most accurate approach would be a customized one by the user, but we are not there yet.

Also, the implementation for this approach looks pretty simple, and seems easy to explain to users.

I think we can start by using simple logic we know and control like stable and latest versions.

[humitos]

I still think there is more was could do to give the user the ability to tell us what is latest

I like this idea if we can allow them to tell this us in an automated way. Otherwise, I think it will become "another tool to update each time a new release is done" and make the process a little more tedious. Making this automated somehow is one of the Read the Docs features that I don't want to loose.

[humitos]

issues stemming from the application being not super transparent with how we determine latest/stable

(from readthedocs/readthedocs.org#5319 (comment))

This makes me think again about the logic behind this addon. Maybe making our logic latest/stable dependent is not a good idea if users have problem understanding how we determine those. This is a good point to avoid this pattern maybe.

[agjohnson]

These are all good points.

Does offering a configuration option help at all here?

I don't quite have a strong opinion here yet, but so far I feel:

• An explicit option is best, but agreed we're not there yet
• Semver (without coercion) probably works for most/many cases
• We could fall back to using latest/stable when semver doesn't work
• If the user can configuration this or at least disable the notifications easily, this seems like maybe enough until we do get an explicit option on Version objects

[humitos]

Does offering a configuration option help at all here?

Yes and no. Ideally, the user should be the one that specify "what's the logic behind this". Originally, I thought this feature in a way the user can "overwrite the javascript function" to tell us what's the highest version to decide whether or not show the notification. However, we are not there yet.

I thought a little about this considering "a simple configuration option" but I didn't come up with a great answer that's easy to understand for users. That's why I asked to the user from readthedocs/readthedocs.org#5319 (comment) for ideas.

Semver (without coercion) probably works for most/many cases

Yes and no. The library we are using doesn't consider 3.1 as valid since it requires the patch version to be present:

$ semver --valid "3.1"
$ echo $?
1

$ semver --valid "3.1.0" 
3.1.0

So, we would need to add extra-logic here for these cases and I'm not sure I want to jump into that path 😅

We could fall back to using latest/stable when semver doesn't work

After reading your comment in the other issue, I'm not 100% convinced that I want to use stable/latest if the users are already confused about those terms. However, those are pretty specific to Read the Docs and the whole platform is built on top of them. So, probably worth following those concepts and fix the root confusion issue by clearly document what they mean and how they are defined. Related issues:

• Rename latest readthedocs.org#10674
• Docs: update versions/versioning page readthedocs.org#9572
• How does read the docs detect tags and build /stable/ when triggering via the api? readthedocs.org#6415
• Docs: mention that only semver versions are considered for the warning banner readthedocs.org#8789
• Clarify implicit version automation readthedocs.org#8476
• Allow for stable to be on a rolling release. readthedocs.org#7219
• Check use of latest for latest unreleased versions vs latest stable versions readthedocs.org#6096

[agjohnson]

Yes and no. The library we are using doesn't consider 3.1 as valid since it requires the patch version to be present

Oooh, yeah that is very annoying then.

Originally, I thought this feature in a way the user can "overwrite the javascript function" to tell us what's the highest version to decide whether or not show the notification. However, we are not there yet.

Agreed. This seems like quite advanced usage too, and requires the author customize both JS and HTML to override the version comparison logic. A simple approach would be great here, but it does feel really hard to make something generally useful here.

---

Overall, I feel like automatic (or at least manual to start) flagging of Version objects as outdated feels like the best UX. I don't yet see a good path between what we have now and that UX though. That is, if we implemented a Version.is_outdated tomorrow, I wouldn't build any of the other features for version comparison into the addons library.

[humitos]

I thought a little more about this and I came back to the idea of using stable and latest only for this logic:

• when reading latest a notification to warn the user about that some features may not be available is shown
• when reading a non-stable nor latest version a notification to warn the user about possible reading and old/outdated version is shown

Note that both notification are pretty simple, it does not require external dependencies and it's easy to explain to users. The warning is shown only if stable is an active version 👍🏼

I implemented this idea in #125

[humitos]

Also, I think this idea of following core Read the Docs feature's like stable and latest versions is important here. @agjohnson, following your example, I image we could create an automation rule to define stable based on a custom logic the user can define using the WebUI. Example, following CalVer.

If we ever implement something like that, this logic behind the addons will keep working fine since it's just based on well-known definitions like stable and latest versions 👍🏼

[agjohnson]

I really like the idea of making these automation rules. The UX would be better even if these were fairly hardcoded automation rules to start.

If we consider the latest and stable additions separate, do you think we also want something like Version.is_outdated? I could see exposing an addons configuration option "Show a warning for all versions except stable". But I could also see users wanting more configuration options or granular control over which versions the warning is displayed on too.

[humitos]

If we consider the latest and stable additions separate, do you think we also want something like Version.is_outdated?

I thought about this and I'm not sure. I'm not clearly visualizing the exact workflow for this yet. We should probably talk more about this and define the workflow together.

I could see exposing an addons configuration option "Show a warning for all versions except stable".

This is basically the logic that I just implemented. Besides, my implementation shows a warning on latest as well to warn them about features that may not be released yet.

But I could also see users wanting more configuration options or granular control over which versions the warning is displayed on too.

This use case is the one I would like to not implement (right now, at least) and point users to more advanced configuration using JavaScript on their own. I'm saying this because Version.is_outdated may not be enough for those cases, so I prefer if they can define their own logic as they want.

[agjohnson]

This is basically the logic that I just implemented.

Aye, I'm talking more about the ability to control it with configuration though.

This use case is the one I would like to not implement (right now, at least) and point users to more advanced configuration using JavaScript on their own

Yeah, this is definitely a later feature either way. I'm not super convinced yet on making users do this JS logic on their own, that definitely makes it harder for us to build this into a feature later. That's to say maybe we don't mention anything about customization just yet.

But otherwise I agree and we should continue talking here about next steps. I think starting with basic logic for now is the best place to start, and is an improvement over relying on logic outside our native versioning.

[astrojuanlu]

Would be nice if this was a bit smarter. For example https://docs.kedro.org/en/0.19.0/ is (at the time of writing) exactly the same as https://docs.kedro.org/en/stable/, but the former shows the "This may be an old version" banner.

Should I open a new issue?

[humitos]

@astrojuanlu we already have an issue to track this at #132 😉