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 API reference snippets #5368

Closed
Youssef1313 opened this issue Aug 27, 2020 · 12 comments
Closed

Fix API reference snippets #5368

Youssef1313 opened this issue Aug 27, 2020 · 12 comments
Assignees
Labels
documentation Related to documentation of ML.NET P2 Priority of the issue for triage purpose: Needs to be fixed at some point.

Comments

@Youssef1313
Copy link
Member

The API reference repository doesn't have issues turned on, so I think it might be suitable here, especially that the broken snippet references used to exist in this repository, but now they don't (or moved somewhere that I can't find).

See dotnet/ml-api-docs#142 for details.

cc: @luisquintanilla

@michaelgsharp michaelgsharp added the documentation Related to documentation of ML.NET label Aug 27, 2020
@luisquintanilla
Copy link
Contributor

Hi @Youssef1313

Could you please provide more clarification where you were seeing these build warnings and which files are being affected in Docs. Is it just the API docs or the conceptual docs as well?

@Youssef1313
Copy link
Member Author

Youssef1313 commented Aug 28, 2020

@Youssef1313
Copy link
Member Author

The problem is this folder doesn't contain a folder called "Static". It was deleted at some point.

The last release branch that contains this folder is: https://github.com/dotnet/machinelearning/tree/release/1.2/docs/samples/Microsoft.ML.Samples

@luisquintanilla
Copy link
Contributor

Thanks for the clarification.

Build Report

@michaelgsharp
Copy link
Member

@Youssef1313 thanks for finding this. After discussing this with @luisquintanilla it appears like when we are building our documentation, the build is always trying to reference the latest branch. This is causing the issues that you pointed out in the PR you created because those files have been removed.

Fixing it manually won't resolve the issue as this situation will continue to happen as code is moved and documentation is regenerated. We need to actually fix the issue in the documentation generation process itself. Unfortunately, I don't know enough about that to resolve it or even have an estimate on how hard that would be to do. I will need to discuss with others on my team about it. So for now, I am going to tag this as a bug and leave it open so we can keep track of it.

@michaelgsharp michaelgsharp added the P2 Priority of the issue for triage purpose: Needs to be fixed at some point. label Aug 28, 2020
@Youssef1313
Copy link
Member Author

Youssef1313 commented Aug 28, 2020

@michaelgsharp You can specify a specific branch. I actually tried that. But another snippets that exists in master and not in release/1.2 generated the same issue.

Basically, there is no "one" branch containing all the snippets. If you can create a new branch, call it documentation for example, and keep it always having all required documentation snippets, that will fix it easily.
Possibly, you can create this branch now from master, and add the current missing folder that exists in release/1.2. Then, we can use the new branch in the doc generation without any problems.

@Youssef1313
Copy link
Member Author

See dotnet/ml-api-docs#143

@michaelgsharp
Copy link
Member

That makes sense. Basically our 2 options are to create 1 branch that has everything, or change the build process to be able to specify a branch for each sample. Does that sound like an accurate statement?

@Youssef1313
Copy link
Member Author

Youssef1313 commented Aug 28, 2020

@michaelgsharp A branch for each sample would be hard I think. (AFAIK, the repo configuration only supports specifying a whole branch external repo referencing - But as I'm not a Microsoft employee, I'm not 100% sure) So, creating 1 branch that has everything is the available easy thing.

@natke natke self-assigned this Sep 9, 2020
@natke
Copy link
Contributor

natke commented Sep 9, 2020

Thank you for raising this @Youssef1313, and for investigating @michaelgsharp and @luisquintanilla.

We decided to host all versions of the ML.NET API docs, but the build system does not support this for previous versions of docs that contain code snippets which do not exist anymore. I think the answer is to only host the docs for the latest ML.NET version. Please let me know if you are aware of any customers who need access the previous versions, or any other reason why we should not take this course of action.

@Youssef1313
Copy link
Member Author

@natke I really don't know whether any customer will need access to the previous versions.

@natke
Copy link
Contributor

natke commented Sep 14, 2020

We've removed the historical preview versions, so this issue is now resolved.

@natke natke closed this as completed Sep 14, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Mar 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
documentation Related to documentation of ML.NET P2 Priority of the issue for triage purpose: Needs to be fixed at some point.
Projects
None yet
Development

No branches or pull requests

4 participants