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

Remove APOC core documentation #3147

Merged
merged 4 commits into from Sep 2, 2022
Merged

Conversation

gem-neo4j
Copy link
Contributor

@gem-neo4j gem-neo4j commented Aug 23, 2022

A lot of deletion 🙃 if you want to see the docs instead do the following from the repo:

cd docs/antora
npm update
npm start

Then open index.html from docs/antora/build/site/apoc/5/index.html

(Note: my Intellij didn't like me doing one then the other, so I had to restart and invalidate caches to get it to work in the other repo)

@gem-neo4j gem-neo4j added 5.0 team-cypher-surface Cypher Surface team should review the PR labels Aug 23, 2022
@gem-neo4j gem-neo4j force-pushed the 5.0_remove_core_documentation branch 2 times, most recently from bfcf6a7 to cc10b05 Compare August 24, 2022 06:51
@gem-neo4j
Copy link
Contributor Author

Split apoc docs

@AzuObs AzuObs self-assigned this Aug 24, 2022
@AzuObs
Copy link
Contributor

AzuObs commented Aug 25, 2022

/installation/index.html

Page contains several references in the text to APOC Core. In general the page might need to be cleaned up because the installation of APOC Extended is fairly different to the installation of APOC Core.

@AzuObs
Copy link
Contributor

AzuObs commented Aug 25, 2022

/overview/index.html

Would it perhaps be nice if this was green like it was before? Doesn't have to be, of course. Also I'm not sure the content of the pill should be that long as it seems to want to warp to a new line (may render differently once CSS rules are fully loaded)

Screenshot 2022-08-25 at 14 35 56

@AzuObs
Copy link
Contributor

AzuObs commented Aug 25, 2022

/virtual-resource/index.html

It seems to me like this whole page could be deleted, because it only contains APOC Core functions

@AzuObs
Copy link
Contributor

AzuObs commented Aug 25, 2022

Some of the comments which I made in the other PR can also be applied to this PR.

  • references to Core functions
  • dbms settings
  • imgs that can be deleted
  • imgs content which should be updated

@AzuObs
Copy link
Contributor

AzuObs commented Aug 25, 2022

I think that it would be nice to leave the Extended docs in an as-nice-as-possible state before handing them over. Users will continue to read them and engineers will need to continue to maintain them.

However, if you feel that some of these would take an inordinate amount, then please feel free to bring that up.

@gem-neo4j
Copy link
Contributor Author

@AzuObs Thank you for the review, I know it was a lot 🥵

I’ve worked through the comments and gone back through the PRs to clean up misses :) With core, it was straight forward to remove all mentions of extended, however in extended I found that removing all mentions of any core procedure/function would remove the usability of the examples in the docs, so instead of removing them, if they are mentioned in examples, I have left it with the assumption that the user will also need core installed (or at least can understand what the example is doing).

In regards to the pills, I don’t know how the colour works, it seems to be decided by the name (Nacho changed them to extended from full, hence the colour change). One option is we remove them? As they don’t give much more info, but that could be confusing, so am not sure what to do there. (Couldn’t find css that worked with them, but also my web developer skills are zero, so idk :P )

I agree that the docs should be nice, sadly they weren’t nice to begin with, so not sure how much effort to put in here 🤔

For answers on core ones (different PR but easier to have all responses together maybe). In terms of videos, I will leave them as is atm, same with retaking screenshots, just until I have met more with the docs team about the move (so as to not do work that may be irrelevant later). I think one thing to think of here is that the core PR is an intermediate step :)

I would like to discuss as a team about the communication of the removal of apoc, and also similar to previous comment, I feel like adding things in this PR doesn’t make much sense, as this isn’t the new docs (most likely), so might be done after.

I am happy with the decision to remove the dv docs :)

@gem-neo4j
Copy link
Contributor Author

An update as well, I investigated more mistakes in the docs (the extended/core is the place it should say, but doesn't in the released docs). I have updated these in my PRs to reflect the accurate one.

apoc.bolt.execute — extended
apoc.get.rels — core
apoc.nlp.aws.* — extended
apoc.systemdb.export.metadata — extended

@AzuObs
Copy link
Contributor

AzuObs commented Aug 30, 2022

Hi Gem, thanks for the message - I've read it and it looks good. I don't really have anything to discuss further in that area, except perhaps your question about the pills where I think I agree that they could be entirely removed from the docs since they don't make much sense anymore.

I will shortly be going over these PR again in a bit more detail 🙃

@AzuObs
Copy link
Contributor

AzuObs commented Aug 30, 2022

5/installation/index.html#neo4j-server

I'm not sure how the new maintainers of this repository will want to version the product, and whether they will have a compatibility layer to ensure backwards compatibility. It would be strange for APOC Extended to be using a different versioning scheme than APOC Core, though... They'd probably want a pre-5.0 section and a post-5.0 section...

If we have some things we weren't able to do in this repository because of time, then maybe we could make a ticket for the new maintainers with a list of things we recommend they look into?

@AzuObs
Copy link
Contributor

AzuObs commented Aug 30, 2022

I had previously made this comment

/virtual-resource/index.html
It seems to me like this whole page could be deleted, because it only contains APOC Core functions

This was before I knew that apoc.dv.* was in fact miscategorised in the docs, and that it is in fact part of APOC Extended. As such I think we ought to bring this page back. WDYT?

@AzuObs
Copy link
Contributor

AzuObs commented Aug 30, 2022

The second round of PR review for this PR is now also done :) Thanks for all the hard work

@gem-neo4j
Copy link
Contributor Author

Haha oh dear :P I can for sure bring the file back 😅

Thanks for all the reviewing 💪

Copy link
Contributor

@AzuObs AzuObs left a comment

Choose a reason for hiding this comment

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

LGTM 🎉 Great job!

@gem-neo4j gem-neo4j merged commit b96fea8 into dev Sep 2, 2022
@gem-neo4j gem-neo4j deleted the 5.0_remove_core_documentation branch September 2, 2022 10:02
vga91 pushed a commit that referenced this pull request Sep 2, 2022
* Remove core documentation

* Remove missed core documentation and images

* Remove all extended pills as no longer useful for the docs

* Bring back apoc.dv
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
5.0 team-cypher-surface Cypher Surface team should review the PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants