generated from lit/lit-element-starter-ts
-
Notifications
You must be signed in to change notification settings - Fork 30
feat: correction of navbar links in docs.jenkins.io #164
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
Open
biru-codeastromer
wants to merge
16
commits into
jenkins-infra:main
Choose a base branch
from
biru-codeastromer:navbar-links
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 3 commits
Commits
Show all changes
16 commits
Select commit
Hold shift + click to select a range
efffa55
feat: correction of navbar links in docs.jenkins.io
biru-codeastromer 0e038d5
fix lint
biru-codeastromer 4be51c4
Merge branch 'main' into navbar-links
krisstern be1c1e7
Merge branch 'main' into navbar-links
krisstern 4c7122d
Merge branch 'main' into navbar-links
krisstern 5249f94
Merge branch 'main' into navbar-links
biru-codeastromer 8d3817e
new updates to handle version
biru-codeastromer 1fe3fe0
fix lints
biru-codeastromer 6994dd9
Update jio-navbar.ts
biru-codeastromer 354928e
Update jio-navbar.ts
biru-codeastromer dedcc6a
fixing
biru-codeastromer 80ea232
new dynamic approach
3a9b291
new approach
biru-codeastromer 51e2efa
Merge branch 'main' into navbar-links
biru-codeastromer ddea1ff
fix
biru-codeastromer a477dec
Merge branch 'navbar-links' of https://github.com/biru-codeastromer/j…
biru-codeastromer File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will be a list/array in the near future. Should be make provisions to prepare for that eventuality?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the feedback! Great point for future
How about we update docsVersion to docsVersions and convert it to an array type. I will make sure the implementation defaults to the current version but remains scalable to support multiple versions in the future.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we can do this in the property defintion -
and then modify the getDocsUrl method to :
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But it wont have that version selection ability , so maybe I will see now how i can modify the code to have that ability , maybe this PR we should like keep the review on hold for sometime and come back to this when we successfully complete our new side navbar implementation in antora .
What you think about it?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes the best time for it will be for me to continue finding currently the best way to achieve version selection logic and get it done so you all can review after as i mentioned that new side bar implementation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, take sufficient time on thinking about this before deciding on a solution!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@biru-codeastromer any update on this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we could automate the updating of this part too. What would you recommend us do here @gounthar?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@krisstern Hey , as discussed in our weekly meet, this is one of this week's task . After I complete the pages migration I will finish this part + some small changes in antora for tomorrow radar
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By tomorrow, I believe Bruno reply will also finalise our changes needed + I will inform before I start how I want to update this PR in slack