- 
                Notifications
    
You must be signed in to change notification settings  - Fork 32
 
Initial support for cross link resolving #491
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
Conversation
| var dictionary = new Dictionary<string, LinkReference>(); | ||
| foreach (var link in _links) | ||
| { | ||
| var url = $"https://elastic-docs-link-index.s3.us-east-2.amazonaws.com/elastic/{link}/main/links.json"; | 
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.
I think this can be master in some cases
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.
Aye, that's why we need a links index file that points to all current index files.
| { | ||
| var url = $"https://elastic-docs-link-index.s3.us-east-2.amazonaws.com/elastic/{link}/main/links.json"; | ||
| _logger.LogInformation($"Fetching {url}"); | ||
| var json = await client.GetStringAsync(url); | 
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.
What happens if the url doesn't exit yet?
While we onboard repositories, not all of them might be available from the start.
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.
For now I'm only going to make docs-content mandatory to resolve.
All others we'll blindly transform to urls for now. (will follow up with another PR to introduce this behavior).
This is the first stab at it:
All links resolve to:
https://docs-v3-preview.elastic.dev/elastic/<repos>/tree/maincurrently.The index.json file should also list the
currentbranch name so we do not assumemain.