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

Autogenerate Java docs (II) #38

Merged
merged 1 commit into from
Oct 20, 2020
Merged

Conversation

dgarcia360
Copy link

Summarizes all commits from #29 in just one commit

@dgarcia360 dgarcia360 mentioned this pull request Sep 28, 2020
@haaawk
Copy link

haaawk commented Sep 28, 2020

This looks mostly good to me but it's also very invasive and will probably generate lots of merge conflicts once we sync with the upstream.

  1. Are all those changes necessary? @tzach
  2. What's the procedure to use those scripts?

@dgarcia360
Copy link
Author

@@ -1,3 +1,7 @@
```eval_rst
:orphan:
```
Copy link

Choose a reason for hiding this comment

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

Are this addition a must?
It will make sync with upstream hard
If no other way, lets disable the warning it generate

Copy link
Author

@dgarcia360 dgarcia360 Sep 30, 2020

Choose a reason for hiding this comment

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

It's not a must, but Sphinx throws the warning WARNING: document isn't included in any toctree. This does not stop the documentation from the building if deleted.

@tzach
Copy link

tzach commented Sep 29, 2020

@haaawk The goal is making the MD file update minimal to make sync with upstream easy
We are almost there

@haaawk
Copy link

haaawk commented Sep 30, 2020

@haaawk How to build the docs is explained in this file https://github.com/scylladb/java-driver/pull/38/files#diff-3160134a4c3b6772a52ddbc542e75d2f

Ok. I asked the wrong question. What do I have to do to cherry-pick this commit to a new branch if I need to (and I know for sure I will have to do this many times)?

faq/osgi/README.md Outdated Show resolved Hide resolved
@dgarcia360
Copy link
Author

dgarcia360 commented Sep 30, 2020

@haaawk I'd like to understand the process you follow to pull the latest changes from upstream while keeping all the modifications you applied before. Could you share with me the steps you follow in the terminal? With this info, I could write some instructions showing how we can apply the docs generation to the next releases. Thanks!

@haaawk
Copy link

haaawk commented Sep 30, 2020

@haaawk I'd like to understand the process you follow to pull the latest changes from upstream while keeping all the modifications you applied before. Could you share with me the steps you follow in the CLI? With this info, I could write some instructions showing how we can apply the docs generation to the next releases. Thanks!

Thanks @dgarcia360. So far I was only doing this with the Java code and the process is as follows:

  1. I have an old branch which is based on some upstream branch. Let's call the branch 3.7.1-scylla and the upstream branch 3.7.1. This branch has some Scylla specific modifications.
  2. When there's a new release of the upstream. For example 3.8.0. We fork it, create new branch called 3.8.0-scylla and cherry-pick all the Scylla specific modifications to it.

I use git cherry-pick to cherry pick the changes. Most of the time they apply cleanly but sometimes there are merge conflicts which I resolve manually. I know how to do that because it's just code.
Now the question is what can I do for documentation in case of conflict.
One option is resolving every conflict manually. I can do it for docs but I'm not sure about the scripts.
Is there any other option?

@dgarcia360
Copy link
Author

dgarcia360 commented Oct 1, 2020

Thanks @haaawk. The problem with the current process applies to code and docs in the same way. If you have to edit the custom code, you would have to cherry-pick multiple commits in order to apply the changes to newer versions, which will become complex to manage if many changes are added.

I propose to merge all changes in one integration branch (e.g. latest). As far as I know, other drivers like https://github.com/scylladb/python-driver/ already follow this approach.

  • If there is a custom modification, you can add it to the integration branch without having to cherry-pick this commits in the future.
  • If there is a new upstream release, you can move all the changes from their version branch into your latest with git merge upstream/<version-name>. This will require resolving all the merge conflicts that might arise due to the modifications.

Once the branch is in a status to be released, you can either create a new branch from the integration branch and tag it with the version number or create a GitHub release.

Please let me know if this new process could work in this repository. If not, the other alternative would be to cherry-pick this PR, which has already been squashed in commit, and pick other future docs modifications as now is being done with code changes.

@dgarcia360
Copy link
Author

I had a conversation today with @fruch to know more about how he is managing the python-driver releases. I can confirm that the python driver merges all the changes into an integration branch (master) to advance on the documentation efforts and other custom code added to the plugin advance if necessary.

@haaawk Here is the script used by the python-driver to merge the next tag from the upstream. Could you please review it and evaluate if it would be possible to use the same approach for the java-driver?

I'm totally aware that this would require some extra efforts to merge new releases that might not have been originally scoped in the short term. However, forks are normally created to extend or adapt the functionality, fix some bugs, or add additional value. Looking into the future, and assuming that new changes would be added as the time passes (code and docs), it would cause even fewer conflicts integrating the latest commits into an integration branch instead of cherry-picking 100s of commits.

We could also use the script created by @fruch to set an internal standard to work with projects depending on upstream repos. Consistency fastens the onboarding of new contributors to the driver's projects, and the integration can happen regularly since downloading the latest changes can be partially automated.

Other solutions have been discussed, like treating docs generation exclusively in a separate orphan branch or abstracting the docs generation in a separate repository. The first would require to deal with merge conflicts every time we want to update the documentation branch, deferring but not solving the problem to treat with merges. The second would work well to generate docs, but editing any documentation file (e.g. Rellabellign DatasTax -> ScyllaDB) would not be possible without editing this repository.

@haaawk
Copy link

haaawk commented Oct 12, 2020

@fruch how is this script working for you? I can see that the last changes pulled from upstream in python driver were on 26th Feb 2020. Documentation changes were done only after that. Have you tried running the script after documentation changes? How many conflicts did it generate?

I'm a bit sceptical to the integration branch idea. We have a few changes in the driver we maintain while the upstream is actively developed by a whole team of people. The tradeoff is definitely in favor of cherry-picking a few commits than resolving conflicts when merging 100s of new commits coming from the upstream.
The integration branch may work for python because it has way fewer active contributions:
python
image
java
image
Moreover, Java driver is released multiple times a month and have two flavors 3.x and 4.x
image
While Python driver is released much less frequently
image

Thus, I'd rather have docs generation on a separate branch or in a separate repository.
Another problem is that users sometimes need Scylla Java driver based on a very specific version of upstream. The integration branch does not allow it.

@fruch
Copy link

fruch commented Oct 12, 2020

@haaawk I'm not sure I'm following, you are reviewing each patch an cherry pick them one by one into a branch ? And anyhow you need to solve call conflict if there are any...

We decided to keep releasing version based on the upstream versions, and we only have patch version in between.

As for custom versions, we are not yet been asked for such things, so far people were taking only one the versions we released.

@haaawk
Copy link

haaawk commented Oct 12, 2020

@fruch When I create new release based on some upstream branch X, I first create a new branch based on X and then I cherry-pick our Scylla-specific changes on top of it.

@fruch
Copy link

fruch commented Oct 12, 2020

@fruch When I create new release based on some upstream branch X, I first create a new branch based on X and then I cherry-pick our Scylla-specific changes on top of it.

so have 1 or 2 of those for docs, and it's basically the same.

I think docs should be first class citizen in a code repo.

docs/source/installation.md Outdated Show resolved Hide resolved
docs/source/installation.md Outdated Show resolved Hide resolved
docs/source/installation.md Outdated Show resolved Hide resolved
docs/source/installation.md Outdated Show resolved Hide resolved
docs/source/index.rst Outdated Show resolved Hide resolved
docs/source/index.rst Outdated Show resolved Hide resolved
@haaawk
Copy link

haaawk commented Oct 12, 2020

@fruch When I create new release based on some upstream branch X, I first create a new branch based on X and then I cherry-pick our Scylla-specific changes on top of it.

so have 1 or 2 of those for docs, and it's basically the same.

It's not. I'm capable of resolving conflicts in code but I know nothing about this docs generation mechanism.

I think docs should be first class citizen in a code repo.

Sure but automation like this needs to be maintained by someone who actually understands what it does. Who's it gonna be? @lauranovich ?

@haaawk
Copy link

haaawk commented Oct 12, 2020

I just tried to cherry-pick this PR on top of 4.x:

$ git status
On branch test
Your branch is up to date with 'origin/4.x'.

You are currently cherry-picking commit e0e51e185.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	new file:   .github/workflows/pages.yml
	new file:   README-dev.rst
	new file:   docs/Makefile
	new file:   docs/_utils/deploy.sh
	new file:   docs/_utils/doxygen.sh
	new file:   docs/_utils/multiversion.sh
	new file:   docs/_utils/redirect.sh
	new file:   docs/_utils/redirections.yaml
	new file:   docs/_utils/setup.sh
	new file:   docs/poetry.lock
	new file:   docs/pyproject.toml
	new file:   docs/source/api.rst
	new file:   docs/source/changelog
	new file:   docs/source/conf.py
	new file:   docs/source/faq
	new file:   docs/source/index.rst
	new file:   docs/source/installation.md
	new file:   docs/source/manual
	new file:   docs/source/upgrade_guide

Unmerged paths:
  (use "git add/rm <file>..." as appropriate to mark resolution)
	both modified:   .gitignore
	both modified:   changelog/README.md
	deleted by them: docs.yaml
	deleted by us:   doxyfile
	both modified:   manual/README.md
	deleted by us:   manual/object_mapper/README.md
	deleted by us:   manual/statements/README.md
	both modified:   upgrade_guide/README.md
	deleted by us:   upgrade_guide/migrating_from_astyanax/README.md

What am I suppose to do with deleted by us: doxyfile?

@fruch
Copy link

fruch commented Oct 12, 2020

@fruch When I create new release based on some upstream branch X, I first create a new branch based on X and then I cherry-pick our Scylla-specific changes on top of it.

so have 1 or 2 of those for docs, and it's basically the same.

It's not. I'm capable of resolving conflicts in code but I know nothing about this docs generation mechanism.

I think docs should be first class citizen in a code repo.

Sure but automation like this needs to be maintained by someone who actually understands what it does. Who's it gonna be? @lauranovich ?

It's called sphinx and it's the same one used across all our docs (from scylla core ones, to python-driver)
@dgarcia360 expertise was called for this purpose

So the answer who maintains it in all those other places would help, plus I can give a hand with it if needed, I have some mileage with it.

@dgarcia360 dgarcia360 force-pushed the docs-automation-2 branch 3 times, most recently from a377d24 to 32276df Compare October 13, 2020 11:27
@dgarcia360
Copy link
Author

The latest commit imports the README contents from the original file https://github.com/scylladb/java-driver/pull/38/files to have fewer changes.

Thus, I'd rather have docs generation on a separate branch

Ok, let's start by integrating the changes into a separate branch. At least we will be able to serve the docs for the current version while keeping the door open to move to the integration branch approach in future iterations. Once there are new upstream releases, we will be able to test in this branch how difficult is to merge the changes in the documentation branch and evolve the solution from there.

@Haawk Could you please create a new branch named docs aligned with the latest version? I'll move the base and edit the code to publish the docs from this new branch.

@haaawk
Copy link

haaawk commented Oct 13, 2020

Sure. Will do.

@dgarcia360
Copy link
Author

Thanks!

@lauranovich
Copy link

@haaawk let me know when you want me to step in - @dgarcia360 what else remains?

@haaawk
Copy link

haaawk commented Oct 13, 2020

The branch is here https://github.com/scylladb/java-driver/tree/docs

@dgarcia360 dgarcia360 requested review from tzach and haaawk October 13, 2020 13:38
@haaawk
Copy link

haaawk commented Oct 13, 2020

Should we merge this PR against docs branch instead of latest?

@dgarcia360 dgarcia360 changed the base branch from latest to docs October 13, 2020 13:52
@haaawk
Copy link

haaawk commented Oct 14, 2020

I talked to @lauranovich and we came to the conclusion that we can try to accept this PR on the release branch. There are 2 things I need to ask you @dgarcia360 though:

  1. Please make sure that you don't override changes you do in one commit in the following commit. This will make my life easier when I will be cherry-picking your commits to the new branches
  2. Please prepare one version of this PR for 3.10.2-scylla and another for 4.9.0-scylla. The two lines (3.x and 4.x) differ significantly but inside a single line, I should be able to manage conflicts.

Thanks,
Piotr

@dgarcia360
Copy link
Author

dgarcia360 commented Oct 15, 2020

I talked to @lauranovich and we came to the conclusion that we can try to accept this PR on the release branch

Nice! I rolled back the commit and moved the base to the latest branch.

  1. Please make sure that you don't override changes you do in one commit in the following commit. This will make my life easier when I will be cherry-picking your commits to the new branches

Configuration-wise, I don't expect to add more changes other than minor commits to update the theme's version and to list new supported documentation versions in the file conf.py from time to time.

Based on other projects we have added docs, I don't think that the documentation contents will be kept static. At least some links pointing to upstream docs and texts will need edits by you and @lauranovich in future PRs.

2.Please prepare one version of this PR for 3.10.2-scylla and another for 4.9.0-scylla. The two lines (3.x and 4.x) differ significantly but inside a single line, I should be able to manage conflicts.

Ok, once the PR is merged I'll submit a couple of PRs to these specific branches by cherry-picking this commit. Let me know if you want to try yourself instead and do not hesitate to share with me any issues that may arise.

Update README-dev.rst

Prefer warnings

Updated CHANGELOG

Import README from original file

Rollback spaces

Fix spaces

Fix spaces

Added 404

Removed  pipx
@haaawk haaawk merged commit 6998ebd into scylladb:latest Oct 20, 2020
@dgarcia360 dgarcia360 deleted the docs-automation-2 branch October 20, 2020 09:42
annastuchlik pushed a commit to annastuchlik/java-driver that referenced this pull request Aug 2, 2023
The control connection does not reconnect anymore when the protocol version 
is negotiated and downgraded, see JAVA-2473. This test was still expecting a
reconnection, and thus was failing since.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants