Skip to content
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

Move top-level VCS fields reference page content back to description pages #10543

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

malteneuss
Copy link
Collaborator

A small improvement of a step of the user guide improvement initiative #9214:

I removed the top-level VCS fields reference page and moved its content into the .cabal and cabal.project reference pages with a bit of duplication. With more content coming to the Cabal documentation, the top-level space is precious and a short description of a technical detail doesn't feel to pull its weight.

  • Patches conform to the coding conventions.
  • Is this a PR that fixes CI? If so, it will need to be backported to older cabal release branches (ask maintainers for directions).

doc/cabal-package-description-file.rst Outdated Show resolved Hide resolved
doc/cabal-project-description-file.rst Outdated Show resolved Hide resolved
@geekosaur
Copy link
Collaborator

I am not sure I agree with removing the VCS fields section entirely, but I do agree it is a poor fit. This may mean we want to split the user manual from a technical reference, with links thereto as needed.

@malteneuss malteneuss force-pushed the remove-version-control-fields-page branch from d7e3854 to 73589bf Compare November 10, 2024 21:20
@malteneuss
Copy link
Collaborator Author

The content of that section isn't gone. It just now lives in the two reference pages with a bit of duplication, which i think is fine, because the use cases are quite separate. A nice guide how to fill those attribute is also still present.

@philderbeast
Copy link
Collaborator

Would it be possible to avoid the duplication and remove the version-control-fields from the table of contents?

One way to do this would be:

$ git diff
diff --git a/doc/index.rst b/doc/index.rst
index 0b5407e85..0791253d8 100644
--- a/doc/index.rst
+++ b/doc/index.rst
@@ -27,7 +27,6 @@ Welcome to the Cabal User Guide
 
    cabal-package-description-file
    cabal-project-description-file
-   version-control-fields
    cabal-config-and-commands
    external-commands
    setup-commands
diff --git a/doc/version-control-fields.rst b/doc/version-control-fields.rst
index 739905d4b..33ba9046f 100644
--- a/doc/version-control-fields.rst
+++ b/doc/version-control-fields.rst
@@ -1,3 +1,5 @@
+:orphan:
+
 Version Control System Fields
 =============================

@malteneuss
Copy link
Collaborator Author

I would like to avoid having a page that is basically only reachable via a an anchor link deep in the reference pages, and which creates a layer of indirection to read the full reference content. This small amount of duplication in the field descriptions shouldn't be a problem as it's quite stable; before your improvements those sections only ever where touched 2x for tiny changes in 4 years.

@philderbeast
Copy link
Collaborator

Would it be possible to avoid the duplication

What about using other means to avoid duplication such as the include directive?

@ulysses4ever
Copy link
Collaborator

Thanks for the contribution! I definitely feel that it's the right direction. I only will be able to properly review this in the matter of days (due to the ongoing release procedures and last minute issues related to this like #10548).

@philderbeast philderbeast mentioned this pull request Nov 17, 2024
2 tasks
@Mikolaj Mikolaj self-requested a review November 20, 2024 10:58
**source code repository**.
A metadata marker that helps users of your package find its source code.
This marker points to a *source code* snapshot within a
version control systems (VCS) **repository** like Git.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
version control systems (VCS) **repository** like Git.
version control systems (VCS) repository like Git.

I really don't see a need for bold here. But that's subjective.


There are two kinds. You can specify one or the other or both at once:
Here are some examples that point to snapshots of the Cabal library in
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think moving the examples before the definition of the kinds (this vs head) is a bad idea: without knowing about this and head examples make little sense.


.. pkg-field:: type: VCS kind
.. pkg-field:: type: VCS tool
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think tool is a good word here. Kind is better but it's overloaded in this section referring to this or head. I'd use "type" for one of the cases and "kind" for another.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about "The VCS hosting the repository"?

Copy link
Collaborator

@ulysses4ever ulysses4ever Nov 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like it has to be one word. I also don't think "hosting" is a good match: I'm used to think of hosting in the context of VCS as the github hosting, gitlab hosting and others. So, to me using the word hosting here would be confusing.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A version control system is a software tool that automates version control.
wikipedia/version_control

Should we shorten all of the VCS references to VC? If so then this change would be (otherwise it's VCS system):

- .. pkg-field:: type: VCS kind
+ .. pkg-field:: type: VC system

The other thing we could do is drop the widespread use of VCS as an acronym. "Version Control" is not too long.

@@ -53,8 +53,9 @@ for a package, both for :ref:`package authors<pkg-authors>` and maintainers and
for :ref:`package consumers<pkg-consumers>`.

A relatively structured set of version control system (VCS) fields, that vary
depending on the :ref:`VCS kind<vcs-kind>`, enables Cabal commands and other
tools to interpret and make effective use of this information.
depending on the :ref:`VCS tool<source-repository-package-type>`,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here the link says "type", which makes much more sense than "tool"

@philderbeast
Copy link
Collaborator

I was initially hesitant to use Depends-on:, #10553 (comment), but have had a change of heart. Make Child Pull Requests Dependent on Parent Pull Requests, a long discussion about this, is unresolved and I am unsure about how to go about marking one PR as dependent on another. Can we please mark this PR as dependent on #10553?

@geekosaur
Copy link
Collaborator

Can we please mark this PR as dependent on #10553?

Done (see the initial comment, it's quite simple).


.. cfg-field:: tag: VCS tag

This field is optional.
This field is optional but, if given, specifies a single subdirectory.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a copy and paste typo? Looks like subdir content within the tag field section.


.. pkg-field:: type: VCS kind
.. pkg-field:: type: VCS tool
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A version control system is a software tool that automates version control.
wikipedia/version_control

Should we shorten all of the VCS references to VC? If so then this change would be (otherwise it's VCS system):

- .. pkg-field:: type: VCS kind
+ .. pkg-field:: type: VC system

The other thing we could do is drop the widespread use of VCS as an acronym. "Version Control" is not too long.


This field is required.
- ``git``
Copy link
Collaborator

@philderbeast philderbeast Nov 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about just git with a redirection to other much less popular ones on a dedicated VCS page? 😈

All of the examples for package and project VCS use are for Git. It is the dominant VCS so that is fine.


.. _source-repository-package-type:

.. cfg-field:: type: VCS tool
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we change this to vcs-field? As can be seen in the diff in #10553 (comment), we're using pkg-field and cfg-field to mean the same thing.

-.. pkg-field:: location: VCS location
+.. cfg-field:: location: VCS location

doc/cabal-project-description-file.rst Show resolved Hide resolved
@@ -27,7 +27,6 @@ Welcome to the Cabal User Guide

cabal-package-description-file
cabal-project-description-file
version-control-fields
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Of all the cabal-related things I do, adding a source-repository-package to a cabal.project is the most frequent when maintaining a project. To be able to look up these fields with one click is a convenience I'd not like to loose.

Comment on lines -9 to -36
.. list-table::
:header-rows: 1
:widths: 30 30 40

* - Field Name
- source-repository (head|this)
- source-repository-package
* - type
- [x]
- [x]
* - location
- [x]
- [x]
* - branch
- [x]
- [x]
* - tag
- [x]
- [x]
* - subdir
- [x] (0 or 1)
- [x] (0 or 1 for each dependency)
* - module (CVS only)
- [x]
- [_]
* - post-checkout-command
- [_]
- [x]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be a shame to loose this table, 4 of the fields are the same but 3 are not. It is good to be able to see that.

@philderbeast
Copy link
Collaborator

@malteneuss, I see #10553 has merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants