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.

Sign up for GitHub

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

CASSANDRA-17750: Add docs for dependency management changes #170

Open
wants to merge 13 commits into
base: trunk
Choose a base branch
from
Open

CASSANDRA-17750: Add docs for dependency management changes #170

Changes from 5 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
97d1340
Add docs for dependency management changes in CASSANDRA-17750
aratno Sep 1, 2022
dd7af1c
4.1 -> 4.2
aratno Sep 1, 2022
1b81c58
Expand documentation
aratno Sep 1, 2022
23f2f82
Rm outdated lib steps
aratno Sep 1, 2022
0411c2c
Separate pre-4.2 and post-4.2 documentation per feedback
aratno Sep 1, 2022
ed4d65a
Update intro
aratno Sep 6, 2022
62c7546
Updates based on Claude Warren's feedback
aratno Oct 31, 2023
1591f8e
Fix typo
aratno Nov 1, 2023
c1211af
Apache -> ASF
aratno Nov 2, 2023
1889d09
4.2 -> 5.0
aratno Nov 2, 2023
ed2c1a5
Clarify release process
aratno Nov 3, 2023
8a6bd37
Clarify lib/ and resolver-dist-lib, per PR feedback
aratno Nov 6, 2023
fd4d6f7
Fix link
aratno Nov 6, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 31 additions & 3 deletions site-content/source/modules/ROOT/pages/development/dependencies.adoc
View file
Open in desktop
Copy link
Contributor

Choose a reason for hiding this comment

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

The POM FIle Types section does not make sense where it is now. As it is at the same level as the 4.2 and pre-4.2 sections it would appear to apply to both, however, it mentions files that are only in the pre-4.2 code. I think that a section for the POMs found in each section would be cleaner and easier to understand.

Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,38 @@ As Cassandra is an Apache product, all included libraries must follow
Apache's https://www.apache.org/legal/resolved.html[software license
requirements].

=== Required steps to add or update libraries
=== Dependency management in Cassandra 4.2 and onwards

In Cassandra 4.2 and onwards, dependencies are managed in Maven POM templates
in `.build/*-template.xml`. These templates are processed into valid Maven POMs
with the `ant write-poms` task and copied to `build/*.pom`.
Copy link
Contributor

Choose a reason for hiding this comment

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

This block does not seem to render correctly in the github viewer. That may be an artifact of the viewer. However the * symbols are swallowed in the View File -> Preview mode.


The following POMs from `build.xml` have been migrated to templates:
* `parent-pom` is now at `.build/parent-pom-template.xml`
* `all-pom` is now at `.build/cassandra-deps-template.xml`
* `build-deps-pom` is now at `.build/cassandra-build-deps-template.xml`
Copy link
Contributor

Choose a reason for hiding this comment

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

This block does not seem to render correctly in the github viewer. That may be an artifact of the viewer or there may be a need for a blank line to separate the bulleted list from the proceeding paragraph.

However, I think the sentences should be changed to read:
X is now Y rather than X is now at Y as the later sounds like a file with the name Y will be found in the directory.


To add new dependencies in Cassandra 4.2 onwards, add the dependency to
`parent-pom-template` and `cassandra-deps-template`, and optionally
`cassandra-build-deps-template` if the dependency is required for build only.
See
https://maven.apache.org/guides/introduction/introduction-to-dependency-mechanism.html#Dependency_Management[the
Maven docs] on how to reference dependencies in the parent POM from the child
POMs.
Copy link
Member

Choose a reason for hiding this comment

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

lines 5-9 need fixing. this sentence would be work after that.


For dependency versions that need to be available in `build.xml` and
`parent-pom-template`, specify the version as a property in `build.xml`, add it
to the `ant write-poms` target, then add the property to `parent-pom-template`
with the value of the template substitution.

For more information on this change, see
https://issues.apache.org/jira/browse/CASSANDRA-17750[CASSANDRA-17750].
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
In Cassandra 4.2 and onwards, dependencies are managed in Maven POM templates
in `.build/*-template.xml`. These templates are processed into valid Maven POMs
with the `ant write-poms` task and copied to `build/*.pom`.
The following POMs from `build.xml` have been migrated to templates:
* `parent-pom` is now at `.build/parent-pom-template.xml`
* `all-pom` is now at `.build/cassandra-deps-template.xml`
* `build-deps-pom` is now at `.build/cassandra-build-deps-template.xml`
To add new dependencies in Cassandra 4.2 onwards, add the dependency to
`parent-pom-template` and `cassandra-deps-template`, and optionally
`cassandra-build-deps-template` if the dependency is required for build only.
See
https://maven.apache.org/guides/introduction/introduction-to-dependency-mechanism.html#Dependency_Management[the
Maven docs] on how to reference dependencies in the parent POM from the child
POMs.
For dependency versions that need to be available in `build.xml` and
`parent-pom-template`, specify the version as a property in `build.xml`, add it
to the `ant write-poms` target, then add the property to `parent-pom-template`
with the value of the template substitution.
For more information on this change, see
https://issues.apache.org/jira/browse/CASSANDRA-17750[CASSANDRA-17750].
* Update dependency in `.build/parent-pom-template.xml` with version
** Update `.build/cassandra-deps-template.xml` for runtime and test dependencies
** Update `.build/cassandra-build-deps-template.xml` for build dependencies

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it's important to spell this out in a little more detail, at least while the change is new. I'd support reducing it in the future, when people are more familiar with the process. But right now the change is so fresh that a newcomer might not get a good answer even after asking in Slack, and adding a little more detail here could make their lives a little easier.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree with Abe here. Even people long time on the project who don't deal with those things regularly will benefit

Copy link
Contributor

Choose a reason for hiding this comment

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

I think that the process needs to be spelled out because the people reading this will not be the developer that has experience with it. It needs to be clear in a fairly step by step process.

Linking to external documentation is advantageous and while I think we can expect that developers know how to specify a Maven dependency, the link to the Maven documentation should be included.


Copy link
Member

@michaelsembwever michaelsembwever Sep 1, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

i'm going to nit-pick a bit here… forgive me…

but i'm curious whether folk think this page would be more readable if it contains two separate unadulterated sections:

  • Dependency Management 4.2+
  • Dependency Management before 4.2

For example, the Changes to dependency management in Cassandra 4.2 section puts unnecessary cognitive load onto the reader if they just don't care about how it worked before 4.2
The reader just wants to know what to do.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm up for that - how's my last edit? This separates those two sections a bit more clearly. We can always edit based on feedback from more users as well.

Copy link
Contributor

Choose a reason for hiding this comment

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

I like the latest changes, I would probably just put before 4.2 first and then 4.2. POM file types shouldn't be only at the end I think or we need some kind of anchor to them maybe, where we refer to changes to be done to them or what changed in different versions.
Otherwise LGTM, this is more around how to organize to make the flow easier to follow.
Thanks for looking into that!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In my opinion, the main audience for this doc (in the short term) is developers who are familiar with how to add dependencies in the pre-4.2 world, and are trying to figure out how to do things post-17750. In that scenario, I think it makes sense to increase visibility around the changes by having that section come first. Over time, this will change as 4.2 becomes more familiar to more folks.

Copy link
Member

@michaelsembwever michaelsembwever Sep 6, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

In my opinion, the main audience for this doc (in the short term) is developers who are familiar with how to add dependencies in the pre-4.2 world, and are trying to figure out how to do things post-17750.

I would disagree with this statement :) because that method only existed for a year, ref CASSANDRA-16391. There was a much longer precedence before (checking jar and license files into lib/). I do suspect most developers are next to starting from scratch (and we don't add deps that often).

If you want the extra explanation there, I don't object if it appears later. But I'm in the "don't explain, don't complain" camp, where a tl;dr what do i do comes first so I can be quick. (My example was overly concise, and only meant as an example.)

Copy link
Contributor

Choose a reason for hiding this comment

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

I am with Mick here. Also, I don't like the tribe knowledge which I am honestly even sure that it is not the case here. Not many people deal with the dependencies or do it regularly. Also, yes, good point, things changed last year. I prefer to be more noisy and things to be more documented than less.

=== Dependency management before Cassandra 4.2

Before Cassandra 4.2, dependencies were managed in `build.xml` and managed with
Maven Ant Tasks.

* Add or replace jar file in `lib` directory
* Add or update `lib/license` files
* Update dependencies in `build.xml`
** Add to `parent-pom` with correct version
** Add to `all-pom` if simple Cassandra dependency (see below)
Expand Down