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

Fixes #33215 - Registration - link to documentation #8704

Merged
merged 1 commit into from
Aug 12, 2021

Conversation

stejskalleos
Copy link
Contributor

Link to documentation should not be hardcoded
in the React component but should go through LinksController

@stejskalleos
Copy link
Contributor Author

@ShimShtein

@theforeman-bot
Copy link
Member

Issues: #33215

@ShimShtein
Copy link
Member

@tbrisker , @ekohl , any more concerns?


export const docUrl = (foremanVersion) => {
const rootUrl = `https://docs.theforeman.org/${foremanVersion}/`
const section = 'Managing_Hosts/index-foreman-el.html#registering-a-host_managing-hosts'
Copy link
Member

Choose a reason for hiding this comment

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

The link is changing. Is this still safe to cherry pick to 2.5 or would it be a slightly different cherry pick?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are right, this can't be cherry picked to 2.5. But it is not a problem, downstream branch is not rebased anymore over the Foreman 2.5-stable, so there is no need for cherry-pick.

Copy link
Member

Choose a reason for hiding this comment

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

But for users of 2.5 it is weird because that documentation link is broken. So I'd argue it still needs a cherry pick.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

@stejskalleos, We still need to use the links controller in 2.5 for downstream branding

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ShimShtein yop, for downstream 2.5 branch we will cherry-pick it

Link to documentation should not be hardcoded
in the React component but should go through LinksController
@tbrisker tbrisker dismissed their stale review August 12, 2021 10:41

comments resolved

@ShimShtein ShimShtein merged commit 6b9dbc3 into theforeman:develop Aug 12, 2021
@ShimShtein
Copy link
Member

Looks good, merging.

@stejskalleos , don't forget to cherry-pick the correct link to 2.5

@stejskalleos
Copy link
Contributor Author

@ShimShtein Thanks for merge, luckily there is no need to cherry-pick it to 2.5, see my #8704 (comment)

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