r-multiverse / help

Discussions, issues, and feedback for R-multiverse
https://r-multiverse.org
MIT License
2 stars 2 forks source link

Packages without Releases with/without Tags #17

Closed shikokuchuo closed 3 months ago

shikokuchuo commented 4 months ago

This is in continuation of #13 as you resolved that larger problem, but arrow as it is still doesn't build and (along with some other packages without tags) is causing a build error to show at R-universe which may look alarming to novice users.

The entry for Arrow looks like this:

[submodule "arrow"] path = arrow url = https://github.com/apache/arrow branch = r-universe-release subdir = r

The Apache team have a policy of updating the 'r-universe-release' tag with the latest release.

There are also repos with no tags or releases at all. I think the policy needs to be flexible in those cases and take them from the Github CRAN mirror. I mean it is up to the maintainer whether or not they make tagged releases.

wlandau commented 4 months ago

In case it helps to know, I am the one who contributed the entry for arrow.

I was actually planning to write to the arrow team to introduce r-releases and ask if they would be willing to create GitHub releases for the R front-end. Releases are easy to create, certainly much easier than submitting to CRAN (especially given their recent experiences), so I think they may be open to it.

I agree that whether the maintainer wants to use the CRAN mirror or a GitHub release is up to them. It does not matter where release is hosted as long as:

  1. It is the production release, not the development version.
  2. The release is somewhere R-universe can pick up.

However, in these cases I feel it is important to talk to the maintainer if we can. And I hope the creation of a GitHub release is something we can gently request from package maintainers, as long as we do it in a way that does not make it a deal-breaker. Regardless of our r-releases project, the creation of releases is a good documentation practice, and feeding the habit will be a service to the community in itself.

On this, would it be okay to make assert_package() check if a release exists and flag the package for manual review if it does not? I agree that the failing check does not look good, even if it is pretty much innocuous. And the manual review will allow us to add guidance in an automated way, and it will allow us to catch these cases before the bot piles up and merges too many.

wlandau commented 4 months ago

Another way to look at it: if the bot finds a release, then the smooth experience of the automatic merge is a reward. In other words, we enforce positive incentives to create releases.

shikokuchuo commented 3 months ago

Thanks, I was aware you contributed arrow. The ones without releases I think were dependencies of Shiny I added at an early stage when I hadn't yet realised this problem.

I think it makes sense in specific cases such as this, in that they already target R-Universe as a release location and it may actually simplify their workflow.

Equally though, the fact they create a separate tag may mean that they supply a 'special' version to CRAN (I've not determined if this is the case) and it may not work for them to create a single 'release'.

More broadly though, even if you do try to reach out to maintainers, I can see many cases where they will be unwilling to change practice (I can anticipate responses such as 'I've done this for the past 20 yrs, I'm not going to change now', 'x large company does this, if it's good enough for them it's good enough for me', etc. etc.)

So my point was, in the spirit of transparency and to ensure a consistent experience, should we add this to the policy that where releases are not made, the CRAN mirror can be accepted instead (and may be swapped for the official Github once releases appear).

CRAN mirror submissions are already flagged for manual review so I don't think we need to change anything there.

As for submissions where the submitter does not realise the releases are not present, agree that we should add an automated check for this. I think the PR should be directly rejected with the message that quotes the policy about using the CRAN mirror in these cases - in order to minimise the need for manual intervention.

wlandau commented 3 months ago

As for submissions where the submitter does not realise the releases are not present, agree that we should add an automated check for this. I think the PR should be directly rejected with the message that quotes the policy about using the CRAN mirror in these cases - in order to minimise the need for manual intervention.

Great! I am working on a PR.

wlandau commented 3 months ago

Submitted https://github.com/r-releases/r.releases.utils/pull/6 and https://github.com/r-releases/help/pull/19.

GitHub releases are easy to detect, but GitLab is a pain because curl redirects to the sign-in page. I think it wouldn't be a burden to manually review GitLab URLs because there are so few anyway.

wlandau commented 3 months ago

Just created a release for r.releases.utils 0.0.7. I think that and #19 allow us to catch and handle these non-release cases.

shikokuchuo commented 3 months ago

Thanks for addressing these so quickly!

I do want to make the observation that I think it's fine to be opinionated about this at r-releases, as it is a core proposition that everything remains available for reliability, and we don't want to see CRAN dependencies get taken down and disappear. However addressing your comment https://github.com/r-releases/help/issues/17#issuecomment-1975515555 I'd caution against more intrusive measures reaching out to specific maintainers (at least in a blanket way) as I feel it's somewhat a slippery slope - at what stage is it ok to make an intervention.

wlandau commented 3 months ago

However addressing your comment https://github.com/r-releases/help/issues/17#issuecomment-1975515555 I'd caution against more intrusive measures reaching out to specific maintainers (at least in a blanket way) as I feel it's somewhat a slippery slope - at what stage is it ok to make an intervention.

That's a good point, and it would be a time-consuming chore to go seek out individual maintainers anyway. In the future, I think these issues can be sorted out when the PRs are flagged for manual review and the moderator can discuss in the thread?

For the moment, how do you suggest we handle packages like arrow which are currently in our packages.json?

shikokuchuo commented 3 months ago

I think these issues can be sorted out when the PRs are flagged for manual review and the moderator can discuss in the thread?

I think that's the best course of action. We should be respectful of anyone who did not initiate the interaction and it would be completely optional whether a maintainer joins such a discussion or not.

For the moment, how do you suggest we handle packages like arrow which are currently in our packages.json?

Well, now that the policy is clear, I'm ok to use the CRAN mirror as an interim measure to clear the build errors.

shikokuchuo commented 3 months ago

Nice green check mark now at https://r-releases.r-universe.dev/ :)