update rserve documentation #7992
Conversation
There was a problem hiding this comment.
Thanks @donsizemore for the PR. I'm not a user of this particular guide, but from a consistency standpoint it looks like in other places we don't point to a specific branch but instead point to the script within the scripts folder. Ex.:
- https://github.com/IQSS/dataverse/blob/develop/doc/sphinx-guides/source/installation/advanced.rst
- https://github.com/IQSS/dataverse/blob/develop/doc/sphinx-guides/source/developers/making-releases.rst
I think the advantage of this is that it forces the installation admin/installer to go to the specific version that they are installing instead of introducing a potential version mismatch in the guides (as the develop branch in this case may be a different script than the version they are installing).
Am I understanding this correctly and, if so, let me know what you think about the advantages/disadvantages of the different approaches?
|
@djbrooke We did discuss this yesterday, before Don created the final PR. We definitely don't want to point the user to the specific script for this one: They need to download the contents of the entire directory for the whole thing to work. The reason Don created this PR was that a user apparently had just downloaded the script, without downloading the other files, and tried to run it. But I understand that your main point was pointing them to the specific branch, and that is a valid concern. Except for this particular thing - the Rserve configuration - the chances of us having to make any changes there are slim. And the chances of any such changes introducing an incompatibility are slimmer still... I feel it would be ok to point to the master; under the assumption that the currently released version of this config should be stable. But simply replacing that link with the directory path, as in other docs, should also be ok, I think. |
|
Thx @landreev - I'll defer to you and @donsizemore since you're both more familiar here. If we do want to link to the main branch we should just update the PR to point there instead of develop. Once that's done I think it would be OK to approve. Thanks for discussing (yesterday and in this issue)! |
|
As I said yesterday in the DMs between me, Don, and Leonid, I'm fine with what's being proposed (linking to the "develop" branch) but if in the future it turns out we do need to have tighter versioning around these Rserve files ("these are the files that work with for Dataverse 5.6", for example), we could consider adding them to the installation zip (and rewriting the doc, of course). |
|
Yes, I do agree that we probably don't want to point to develop there. Pointing to the master instead was a random idea that I'm not super attached to. So I'm happy to leave it to @donsizemore, whether to just replace it with the |
|
For the record, I agree - if it ever becomes an issue (or if we ever hear from another user about this config step being a problem) - we'll just add these extra files to the installer bundle.
|
There was a problem hiding this comment.
Thanks @donsizemore and @landreev @pdurbin for discussing. LGTM now.
What this PR does / why we need it: currently the Rserve documentation mentions individual scripts used to install Rserve and where to find them, but there is no explicit link or mention that one needs all files in the subdirectory. This PR links to the directory and spells out that one needs copies of all files contained therein.
Which issue(s) this PR closes:
Closes #7893
Special notes for your reviewer: none
Suggestions on how to test this: none
Does this PR introduce a user interface change? If mockups are available, please link/include them here: none
Is there a release notes update needed for this change?: no
Additional documentation: none