-
Notifications
You must be signed in to change notification settings - Fork 696
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Duplicate VCS content #10553
base: master
Are you sure you want to change the base?
Duplicate VCS content #10553
Conversation
6fd625b
to
8afedf8
Compare
The amount of files created here are quite a lot of moving parts to avoid a bit of duplication. And i still see little benefit in a dedicated VCS pag, which is the reason for #10543. But let's see what direction other maintainers prefer. |
To quantify a "bit of duplication", this is what would be duplicated by #10543 by copy and paste or on this pull request by using RST includes (one for each section's content). VCS kind
^^^^^^^^
Cabal supports specifying different information for various common source
control systems. This is the name of the source control system used for a
repository. The currently recognised types are:
- ``darcs``
- ``git``
- ``svn``
- ``cvs``
- ``mercurial`` (or alias ``hg``)
- ``bazaar`` (or alias ``bzr``)
- ``arch``
- ``monotone``
- ``pijul``
The VCS kind will determine what other fields are appropriate to specify for a
particular version control system.
VCS location
^^^^^^^^^^^^
The location of the repository, usually a URL but the exact form of this field
depends on the repository type. For example:
- for Darcs: ``http://code.haskell.org/foo/``
- for Git: ``https://github.com/foo/bar.git``
- for CVS: ``[email protected]:/cvs``
VCS branch
^^^^^^^^^^
Many source control systems support the notion of a branch, as a distinct
concept from having repositories in separate locations. For example CVS, SVN and
git use branches while darcs uses different locations for different branches. If
you need to specify a branch to identify a your repository then specify it in
this field.
VCS tag
^^^^^^^
A tag identifies a particular state of a source repository. The exact form of
the tag depends on the repository type.
VCS subdirectory
^^^^^^^^^^^^^^^^
A field of this type is always optional because it defaults to empty, which
corresponds to the root directory of the repository and is the same as
specifying ``.`` explicitly.
Some projects put the sources for multiple packages inside a single VCS
repository. This field lets you specify the relative path from the root of the
repository to the top directory for the package, i.e. the directory containing
the package's ``.cabal`` file. |
I agree with @malteneuss that top-level real estate is a finite resource. I also agree with him that this PR has quite a lot going on. @philderbeast I am sure you have already thought about it, but it there any way to reduce the number of files to — say — a single |
@ffaf1, are you talking about the "Version Control System Fields" page? |
I'm not overly worried about the number of inclusions per see but I do agree with others that the top-level page doesn't pull its weight. In fact, I argued exactly that on the original PR, and I was quite surprised that it ended up going in but then again: I didn't feel strong enough about it to block it. As for de-duplication, I think the duplication overall is quite small. It also concerns things that don't change much in our experience, so I'd leave it be. If there are more proponents of the de-duplication proposal, I have a couple of suggestions about the directory structure:
|
Yes. |
Introducing this new |
@ulysses4ever, I've done this, putting those files in a |
There are some existing includes. By using the
|
I picked a snippet for each section to allow for all these differences. $ diff cabal-package-description-file.rst cabal-project-description-file.rst --unified
--- cabal-package-description-file.rst 2024-11-20 20:29:46.263245154 -0500
+++ cabal-project-description-file.rst 2024-11-20 20:30:10.032811088 -0500
-.. pkg-field:: type: VCS kind
+.. cfg-field:: type: VCS kind
This field is required.
.. include:: vcs/kind.rst
-.. pkg-field:: location: VCS location
+.. cfg-field:: location: VCS location
This field is required.
.. include:: vcs/location.rst
-.. pkg-field:: module: token
-
- This field is required for the CVS repository type and should not be
- used otherwise.
-
- CVS requires a named module, as each CVS server can host multiple
- named repositories.
-
-.. pkg-field:: branch: VCS branch
+.. cfg-field:: branch: VCS branch
This field is optional.
.. include:: vcs/branch.rst
-.. pkg-field:: tag: VCS tag
-
- This field is required for the ``this`` repository kind.
+.. cfg-field:: tag: VCS tag
- This might be used to indicate what sources to get if someone needs to fix a
- bug in an older branch that is no longer an active head branch.
+ This field is optional.
.. include:: vcs/tag.rst
-.. pkg-field:: subdir: VCS subdirectory
+.. cfg-field:: subdir: VCS subdirectory list
+
+ Look in one or more subdirectories of the repository for cabal files, rather
+ than the root. This field is optional.
+
+ .. include:: vcs/subdir.rst
- This field is optional but, if given, specifies a single subdirectory.
+.. cfg-field:: post-checkout-command: command
- .. include:: vcs/subdir.rst
\ No newline at end of file
+ Run command in the checked out repository, prior sdisting.
\ No newline at end of file I'd tried using |
Actually I'd tried to keep this PR tight, it only does the duplication using includes. One exception to that was moving the VCS page itself out of the "CABAL REFERENCE" in the table of contents. I did that in response to a comment by @geekosaur, #10543 (comment). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's unfortunate that ReST doesn't support ::include
-ing sections, only whole files.
Also, I'll remind you of Depends-on:
in PRs to ensure things are merged in the right order.
Thanks @geekosaur, I didn't know about this. I don't want to impose that on someone else's PR though. |
Was |
Didn't bother with that option as by then I was convinced a simple import without options was so much simpler. |
I can live with the many small snippets files as they are isolated well now, but i'm even less d'accord with making VCS a top-level title. Getting it out of the top-level pages doctree is my main wish here as we already have a long detailed guide page on the VCS topic, with references to the corresponding reference sections (now without duplication). |
b57aab4
to
c6a99e7
Compare
@malteneuss, I've reverted the table of contents commit. This PR makes no change to the VCS top-level title, not in its place in the table of contents nor how it is rendered. I'd like to keep this one about adding (duplicating) VCS content to the package and project pages. You'd like to remove the VCS top-level title. That's what you're proposing in #10543. Can we please keep removal discussions going there and not here? |
bb3efe2
to
f7523bb
Compare
Label merge+no rebase is necessary when the pull request is from an organisation. |
Congrats on good collaboration skills you two! |
- Break VCS sections into files for inclusion - Add VCS kind content to package and project - Add VCS location content to package and project - Add VCS branch content to package and project - Add VCS kind content to package and project - Add VCS subdir content to package and project - Put "is required" first for CVS field - Move VCS out of the "Cabal Reference" - Rename include files to vcs/* - Revert "Move VCS out of the "Cabal Reference""
f7523bb
to
c635ce0
Compare
@mergify refresh |
✅ Pull request refreshed |
I think your rebase yesterday reset the timer. (The reason a rebase after the timer has expired doesn't affect this is that it looks for the |
Duplicate the VCS content by include reuse, avoiding copy and paste, as I suggested in #10543 (comment).
Background
Both package and project pages have content for version control system (VCS) fields.
Note
With #9701 I'd moved VCS field content to its own page, "Version Control System Fields" page. This PR doesn't change that, neither in the table of contents or how that page is rendered.
Include Reuse
I am proposing this change, to create a folder for these includes and in that put
*.rst
snippet content for.. include::
inclusion by other pages. I duplicate the VCS field content, as it exists now, within both package and project pages and retain the "Version Control System Fields" page.Note
Initially, I'd moved the VCS fields page to a new "Version Control Reference" in the table of contents, outside the "Cabal Reference", but have reverted that.
Merge Order
I would like to see this merged before #10543 so that any further changes that are in addition to the duplication of content are proposed on their own.
Warning
Copy and paste duplication (as #10543 proposes) would be vulnerable to the two copies (on the package and project pages) drifting apart. That would be a maintenance burden (if we noticed) or a user let down (if we updated one copy but not the other).