Skip to content

Conversation

@cbuescher
Copy link
Member

Follow up to #28528. The example page links to a project on master which has been
split into two projects.

@cbuescher cbuescher added >docs General docs changes :Core/Infra/Plugins Plugin API and infrastructure v6.0.3 v6.1.4 v6.2.1 labels Feb 6, 2018
@cbuescher
Copy link
Member Author

@tlrx since you created those two new example projects, could you check if linking to those from older 6.x branches looks okay to you? I was suprised that we seem to link to master most previous versions of this doc, maybe we should link to the appropriate minor version instead? This doesn't seem to happen in most of the rest of the documentation though. Thoughts?

@tlrx
Copy link
Member

tlrx commented Feb 6, 2018

@cbuescher Thanks, I didn't see these links were wrong!

@tlrx since you created those two new example projects, could you check if linking to those from older 6.x branches looks okay to you

I still need to backport #28339 to 6.2/6.1/6.0 branches. It think the best it to change the links to point to each corresponding version, I can do it when I backport the PR.

@cbuescher
Copy link
Member Author

@tlrx great, I didn't know there were plans to backport the example split. In that case changing the links to the current branch might be the best option. I see most external links to some repost in the docs going to ".../tree/master/..." though, so maybe we should quickly check with the docs team what the desired behaviour is here. Maybe @clintongormley has an opinion here.

@tlrx
Copy link
Member

tlrx commented Feb 7, 2018

I see most external links to some repost in the docs going to ".../tree/master/..." though, so maybe we should quickly check with the docs team what the desired behaviour is here.

I think we should use a link reference for the github repository, something like:

:elasticsearch-guthub: https://github.com/elastic/elasticsearch/tree/master/

and for all non master branch:

:elasticsearch-guthub: https://github.com/elastic/elasticsearch/tree/v{version}/

I think it should work.

@cbuescher
Copy link
Member Author

I think we should use a link reference for the github repository, something like:

:elasticsearch-guthub: https://github.com/elastic/elasticsearch/tree/master/

and for all non master branch:

:elasticsearch-guthub: https://github.com/elastic/elasticsearch/tree/v{version}/

A problem with that is that {version} includes the patch version, so it would be 6.2.0 but for the links to the repo we would need only the major and minor (6.2). Not sure where that would be added to the docs build.

@clintongormley
Copy link
Contributor

There is a {branch} attribute which will give you what you need.

@cbuescher cbuescher force-pushed the docs-fixurl-plugin-authors branch from 037b6ae to cefa90b Compare February 7, 2018 11:37
@rjernst
Copy link
Member

rjernst commented Mar 11, 2018

@cbuescher Is there anything preventing this from being merged?

Follow up to elastic#28528. The example page links to a project on master which has been
split into two projects.
@cbuescher cbuescher force-pushed the docs-fixurl-plugin-authors branch from cefa90b to 2747d00 Compare March 13, 2018 10:03
@cbuescher
Copy link
Member Author

cbuescher commented Mar 13, 2018

@tlrx I pushed an update that links to the current branch of the example plugins, it can be updated as soon (or if) #28339 gets backported, but will correct the links in the meantime. Can you take another quick look?

Copy link
Member

@tlrx tlrx left a comment

Choose a reason for hiding this comment

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

LGTM, sorry for the delay it took me to approve this.

I initially wanted to backport #28339 so that plugin samples would be the same for all 6.x versions but it has been blocked by released and other stuff.

* a https://github.com/elastic/elasticsearch/tree/{branch}/plugins/examples/script-expert-scoring[Java plugin]
which contains a script plugin.
* a https://github.com/elastic/elasticsearch/tree/master/plugins/examples/meta-plugin[Java plugin]
* a https://github.com/elastic/elasticsearch/tree/{branch}/plugins/examples/meta-plugin[Java plugin]
Copy link
Member

Choose a reason for hiding this comment

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

Note: this must be removed when backporting to 6.1 and 6.0

* a https://github.com/elastic/elasticsearch/tree/{branch}/plugins/jvm-example[Java plugin]
which contains Java code.
* a https://github.com/elastic/elasticsearch/tree/master/plugins/examples/rescore[Java plugin]
* a https://github.com/elastic/elasticsearch/tree/{branch}/plugins/examples/rescore[Java plugin]
Copy link
Member

Choose a reason for hiding this comment

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

Note: this must be removed when backporting to 6.0

@cbuescher
Copy link
Member Author

@tlrx thanks, will remove the above mentioned bullet point when porting this back.

@cbuescher cbuescher merged commit 31e40c8 into elastic:6.2 Mar 15, 2018
cbuescher pushed a commit that referenced this pull request Mar 15, 2018
Follow up to #28528. The example page links to a project on master which has been
split into two projects.
cbuescher pushed a commit that referenced this pull request Mar 15, 2018
Follow up to #28528. The example page links to a project on master which has been
split into two projects.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Core/Infra/Plugins Plugin API and infrastructure >docs General docs changes v6.0.3 v6.1.4 v6.2.1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants