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

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

Open
wants to merge 13 commits into
base: trunk
Choose a base branch
from
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,13 +12,26 @@ As Cassandra is an Apache product, all included libraries must follow
Apache's https://www.apache.org/legal/resolved.html[software license
requirements].

=== Changes to dependency management in Cassandra 4.1
aratno marked this conversation as resolved.
Show resolved Hide resolved

Before Cassandra 4.1, dependencies were managed in `build.xml` and managed with
Copy link

Choose a reason for hiding this comment

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

don't need ,

Maven Ant Tasks. In Cassandra 4.1 and onwards, dependencies are managed in POM
templates in `.build/`. New dependencies should be added to the parent POM
aratno marked this conversation as resolved.
Show resolved Hide resolved
template and necessary child POMs. See
https://issues.apache.org/jira/browse/CASSANDRA-17750[CASSANDRA-17750] for more
information.

Copy link
Member

@michaelsembwever michaelsembwever Sep 1, 2022

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

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.

=== Required steps to add or update libraries

* Add or replace jar file in `lib` directory
* Add or update `lib/license` files
aratno marked this conversation as resolved.
Show resolved Hide resolved
* Update dependencies in `build.xml`
* Update dependencies in `build.xml` (pre-4.1) or POM templates in `.build/` (4.1 onwards)
* If dependencies are managed in `build.xml` (pre-4.1):
** Add to `parent-pom` with correct version
** Add to `all-pom` if simple Cassandra dependency (see below)
* If dependencies are managed in POM templates in `.build/` (4.1 onwards):
** Add dependency to `.build/parent-pom-template.xml` with correct version and optional scope and classifier
** Add dependency to `.build/cassandra-deps-template.xml` and optionally `.build/cassandra-build-deps-template.xml` with references to parent POM

=== POM file types

Expand Down