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

Fix all docs type class and data types links #1120

Closed
raulraja opened this issue Nov 13, 2018 · 22 comments
Closed

Fix all docs type class and data types links #1120

raulraja opened this issue Nov 13, 2018 · 22 comments
Assignees

Comments

@raulraja
Copy link
Member

All type classes and data type in the documentation site should follow the same path as the package they declared in terms of permalinks in the docs.
For example:

arrow.core.Option -> docs/arrow/core/option

All permalinks need to be modified and if possible preserve existing redirects.
This is necessary because we plan on using reflection and other introspection techniques to have some parts of the docs autogenerated at build time. Links for a given data type or type class need to be predictable so the generator can assume those paths to be there.

@Tylos
Copy link
Contributor

Tylos commented Nov 14, 2018

I have been reviewing the issue and wanted to ask a coupe of questions to make sure I got everything right:

  • I understand all the type classes and data classes are listed in those documentation directories. Did I missed anything?
  • In each file I should modify the permalink variable in Jekyll Front Matter header so it matches the class package
  • For redirects, I have checked that github pages already supports a plugin we could use. External references to documentation should keep working. Should I also update the existing relative urls or references in the project to the new permalink or rely on the redirection?

@JorgeCastilloPrz
Copy link
Member

JorgeCastilloPrz commented Nov 14, 2018

  • Not all of them have docs yet, I think. Still this is more about updating the current route the ones available have so we can support access through some reflection techniques based on the same url pattern.
  • The second one is true. Not sure if you also need to modify some links to those in the side menu. @raulraja?
  • I'm not sure what you mean by redirects?

@raulraja
Copy link
Member Author

raulraja commented Nov 14, 2018

By redirects I mean that we may loose traffic and position reworking the links so it'd be nice if the existing ones would redirect to the new links based on the package.seems like @Tylos has a plan for this though.

@Tylos
Copy link
Contributor

Tylos commented Nov 15, 2018

Currently working on a forked version of arrow

You can access the documentation using the new url docs/arrow/core/option
2018-11-15 12-21-07 2018-11-15 12_25_55

Whereas docs/datatypes/option redirects to the aforementioned link
2018-11-15 12-21-07 2018-11-15 12_23_38

There are some issues I will keep investigating. When the user clicks on the lateral menu for sections which perform a redirect, the menu state is lost
2018-11-15 12-27-54 2018-11-15 16_00_37

@calvellido
Copy link
Member

calvellido commented Nov 15, 2018

@Tylos regarding the menu state, that behaviour you are seeing is because it is set at runtime by comparing through JS the current URL with the one considered for the menu entry, which is set through Liquid for each entry in the sidebar:

if ($this.attr('href') === current){

url: /docs/datatypes/option/

<a href="{% if item.url %}{{item.url}}{% else %}#{% endif %}">

Changing the URL in the menu entry might suffice, but I guess that if the URLs could be changing depending on the source code organization, this has to be considered and updated when that happens.

@pakoito
Copy link
Member

pakoito commented Nov 15, 2018

Uh, it'd be great if the menu scrolled to the open position afterwards. It's something that's bugging me for a while :)

@Tylos
Copy link
Contributor

Tylos commented Nov 20, 2018

@calvellido changing the URLs in menu.yml gets it back to work again as you said!

2018-11-20 11-38-27 2018-11-20 11_44_25

Do you think I should swap other internal usages of old link to the new one?

@Tylos
Copy link
Contributor

Tylos commented Nov 27, 2018

Have completed changing the URL from all the classes in modules/docs/arrow-docs/docs/docs/typeclasses and modules/docs/arrow-docs/docs/docs/datatypes, and now the documentation built with #1119 seems to be working

2018-11-27 11-18-57 2018-11-27 11_28_05

However, there are some cases the links are still broken:

  • There are some cases where the class exists in the code, but there is no documentation page created. That leads to a 404 like CoKleisli
  • There are some modules that will also require to be migrated, such as Optics.

Should I open a PR with these changes or should I address some other changes before @raulraja @calvellido? You can check the changes here

@calvellido
Copy link
Member

Nice @Tylos! We are about to hopefully stabilize the CI process, I think after that your PR will come in handy 👍

@Tylos
Copy link
Contributor

Tylos commented Nov 27, 2018

Have you been able to include some link checker into the mix? That would be awesome!

@JorgeCastilloPrz
Copy link
Member

How's this going? Is there an scenario where we could merge this or we need to wait for something?

@raulraja
Copy link
Member Author

@calvellido just fixed the site in case that was a hold up to get this merged in master

@JorgeCastilloPrz
Copy link
Member

Is that fixing the 404 thing? Besides that: what about the module migration required (as for Optics) mentioned by @Tylos?

@raulraja
Copy link
Member Author

So the current docs have now the API docs. It'd be better if we work on moving the docs in the current type classes and data types to the kdocs and we make the API docs kdoc generated URL the main one in the menu. We need to push forward documenting kdoc first and leave web docs only for tutorials. I'll be happy to jump on a call if needed to explain more details

@JorgeCastilloPrz
Copy link
Member

Sorry, is that directly related to this? Ideally we'd move that discussion to a separate issue to work on it and we'd try to unlock this. What's exactly pending here @Tylos?

@Tylos
Copy link
Contributor

Tylos commented Nov 28, 2018

All the type classes and data types that are in the docs modules are migrated Wich was the goal of the issue.

Would need to update the contribute.md as I had to add a Gemfile. Apart from that should be ready to review

Was waiting for CI to be stable as @calvellido mentioned, and resolve if other packages would be required to change. If you prefer it can be moved to another issue

@raulraja
Copy link
Member Author

Yes it is directly related. The Pages where the work of this ticket is going to get done they will disappear. For example the Option data type will point in the menu here https://arrow-kt.io/docs/apidocs/arrow-core/arrow.core/-option/index.html which is driven by kdoc. So there is a work of porting non tutorial related docs to the kdocs

@raulraja
Copy link
Member Author

@Tylos in that case if the work is done it has to be merged now before we start with the migration to get all kdocs properly documented with examples per function.

@JorgeCastilloPrz
Copy link
Member

Sounds good. Let's try to move on this tomorrow or when you have a moment then @Tylos. Let me know if you need any help for unlocking pls. Sorry for being a bit too spammy at this time.

@Tylos
Copy link
Contributor

Tylos commented Nov 28, 2018

Will do tomorrow first thing in the morning

@calvellido
Copy link
Member

There was some gems/jekyll misbehaving but everything is now working, and already merged. If everything goes well on the CI side (it should) you can expect the changes to go live in about ~20 minutes.

Thanks for the hard work @Tylos!

@Tylos
Copy link
Contributor

Tylos commented Nov 29, 2018

🤞 hope everything goes well!

Kudos to @pedrovgs who helped me out with jekyl 💋

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

No branches or pull requests

5 participants