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

[WIP] Build docs using 'fsdocs' tooling #27

Merged
merged 5 commits into from
Aug 1, 2020
Merged

[WIP] Build docs using 'fsdocs' tooling #27

merged 5 commits into from
Aug 1, 2020

Conversation

dsyme
Copy link
Contributor

@dsyme dsyme commented Jul 31, 2020

This is work in progress to use the FSharp.Formatting fsdocs default tooling to build the freshest FSharp.Core docs.

The CI in this PR attempts to do this

  1. build dotnet/fsharp feature/docs branch (where we assume latest doc updates have been pushed)

  2. builds FSharp.Formatting master branch

  3. Uses that FSharp.Formatting tool to build the docs for the FSharp.Core built in step 1

See also README.md

@baronfel We will ened to work together to incorporate any improvements made recently, I need to understand what they are

@dsyme
Copy link
Contributor Author

dsyme commented Jul 31, 2020

@baronfel Is CI set up for this repo at the moment? Or do we need to set it up again?

@baronfel
Copy link
Collaborator

CI is set up to do pushes from master only right now, if we wanted previews of branches we'd need to set that up.

Regarding fixes that happened recently, the only one that's relevant I think is the fix to the search URL path prefixes.

Since the generated site is hosted at a subpath of fsharp.github.io, the path prefix of the site needs to be incorporated into any urls generated by F#F.

@dsyme
Copy link
Contributor Author

dsyme commented Aug 1, 2020

@baronfel @cartermp I have pushed the newly generated docs for FSHarp.Core to https://fsharp.github.io/fsharp-core-api-docs

Could you walk through them and note down everything you see that's wrong with them?

@dsyme
Copy link
Contributor Author

dsyme commented Aug 1, 2020

@Krzysztof-Cieslak would like your opinion too.

@dsyme
Copy link
Contributor Author

dsyme commented Aug 1, 2020

@baronfel I think I'd like to merge this if that's ok. I do think it's the way we need to go.

  • It would be great if you could try out the new workflow in README.md and see about making some iterative improvements to the FSharp.Core docs

  • I'd really appreciate someone with a design eye to take a crack at improving the default temp;ate in FSharp.Formatting. Some small tweaks can make a big difference.

@dsyme
Copy link
Contributor Author

dsyme commented Aug 1, 2020

@baronfel The URLs being produced no longer add FSharp.Core/. We can add that separately. Do you know if that's an issue?

e.g. https://fsharp.github.io/fsharp-core-api-docs/reference/fsharp-collections-arraymodule.html

I wonder if we should rename this repo to api-docs or just docs, put the FSharp.Core back in and then make it a host for a whole swathe of F# library docs.

BTW one concern is that index.json is getting quite large - already 1MB (would compress to 114KB). There's some unnecessary text in there. It's brought to the client side fairly eagerly and that might be a significant perf problem

@cartermp
Copy link
Member

cartermp commented Aug 1, 2020

How much of that data is used to populate tooltips? I think we should aim to reduce the size of the payload, so if that part is significant then it shoul dbe cut. Especially since parameters and return types are already there with each entry.

@dsyme
Copy link
Contributor Author

dsyme commented Aug 1, 2020

How much of that data is used to populate tooltips? I think we should aim to reduce the size of the payload, so if that part is significant then it shoul dbe cut. Especially since parameters and return types are already there with each entry.

It is the search index (we're using a lunr client side index).

I'm not going to prioritise it until we know it's an issue, particularly would like to determine if switching between pages causes a re-download. If it's downloaded only once that's not such a big deal

@cartermp
Copy link
Member

cartermp commented Aug 1, 2020

Gotcha. Looking at the file now

Array.tryPick gives this content:

"Array.tryPick \ntryPick \n<p class='summary'>Applies the given function to successive elements, returning the first\n result where function returns <code>Some(x)</code> for some <code>x</code>. If the function \n never returns <code>Some(x)</code> then <code>None</code> is returned.</p><dl><dt><span class='parameter'>chooser</span></dt><dd><p>The function to transform the array elements into options.</p></dd><dt><span class='parameter'>array</span></dt><dd><p>The input array.</p></dd></dl><p class='returns'><p>Returns: The first transformed element that is <code>Some(x)</code>.</p><table class='inner-list exception-list'><tr><td><a href=\"https://docs.microsoft.com/dotnet/api/system.argumentnullexception\">ArgumentNullException</a></td><td>Thrown when the input array is null.</td></tr></table>"

The module entry gives:

| "Array \n<p class='summary'>Basic operations on arrays.</p><p class='remarks'>\n See also .\n </p> \nArray.Parallel \nParallel \nArray.allPairs \nallPairs \nArray.append \nappend \nArray.average \naverage \nArray.averageBy \naverageBy \nArray.blit \nblit \nArray.collect \ncollect \nArray.compareWith \ncompareWith \nArray.concat \nconcat \nArray.contains \ncontains \nArray.copy \ncopy \nArray.countBy \ncountBy \nArray.create \ncreate \nArray.tryHead \ntryHead \nArray.tryPick \ntryPick \nArray.fill \nfill \nArray.pick \npick \nArray.choose \nchoose \nArray.chunkBySize \nchunkBySize \nArray.distinct \ndistinct \nArray.distinctBy \ndistinctBy \nArray.splitInto \nsplitInto \nArray.empty \nempty \nArray.exactlyOne \nexactlyOne \nArray.tryExactlyOne \ntryExactlyOne \nArray.except \nexcept \nArray.exists \nexists \nArray.exists2 \nexists2 \nArray.filter \nfilter \nArray.find \nfind \nArray.findBack \nfindBack \nArray.findIndex \nfindIndex \nArray.findIndexBack \nfindIndexBack \nArray.forall \nforall \nArray.forall2 \nforall2 \nArray.fold \nfold \nArray.foldBack \nfoldBack \nArray.fold2 \nfold2 \nArray.foldBack2 \nfoldBack2 \nArray.get \nget \nArray.head \nhead \nArray.groupBy \ngroupBy \nArray.indexed \nindexed \nArray.init \ninit \nArray.zeroCreate \nzeroCreate \nArray.isEmpty \nisEmpty \nArray.iter \niter \nArray.iter2 \niter2 \nArray.iteri \niteri \nArray.iteri2 \niteri2 \nArray.last \nlast \nArray.item \nitem \nArray.length \nlength \nArray.tryLast \ntryLast \nArray.map \nmap \nArray.map2 \nmap2 \nArray.mapFold \nmapFold \nArray.mapFoldBack \nmapFoldBack \nArray.map3 \nmap3 \nArray.mapi2 \nmapi2 \nArray.mapi \nmapi \nArray.max \nmax \nArray.maxBy \nmaxBy \nArray.min \nmin \nArray.minBy \nminBy \nArray.ofList \nofList \nArray.ofSeq \nofSeq \nArray.pairwise \npairwise \nArray.partition \npartition \nArray.permute \npermute \nArray.reduce \nreduce \nArray.reduceBack \nreduceBack \nArray.replicate \nreplicate \nArray.rev \nrev \nArray.scan \nscan \nArray.scanBack \nscanBack \nArray.singleton \nsingleton \nArray.set \nset \nArray.skip \nskip \nArray.skipWhile \nskipWhile \nArray.sub \nsub \nArray.sort \nsort \nArray.sortBy \nsortBy \nArray.sortWith \nsortWith \nArray.sortInPlaceBy \nsortInPlaceBy \nArray.sortInPlaceWith \nsortInPlaceWith \nArray.sortInPlace \nsortInPlace \nArray.splitAt \nsplitAt \nArray.sortDescending \nsortDescending \nArray.sortByDescending \nsortByDescending \nArray.sum \nsum \nArray.sumBy \nsumBy \nArray.take \ntake \nArray.takeWhile \ntakeWhile \nArray.tail \ntail \nArray.toList \ntoList \nArray.toSeq \ntoSeq \nArray.transpose \ntranspose \nArray.truncate \ntruncate \nArray.tryFind \ntryFind \nArray.tryFindBack \ntryFindBack \nArray.tryFindIndex \ntryFindIndex \nArray.tryItem \ntryItem \nArray.tryFindIndexBack \ntryFindIndexBack \nArray.unfold \nunfold \nArray.unzip \nunzip \nArray.unzip3 \nunzip3 \nArray.where \nwhere \nArray.windowed \nwindowed \nArray.zip \nzip \nArray.zip3 \nzip3" -- | --

So I think there's definitely room to improve the payload. 1MB isn't too bad I guess, but that is getting into big territory

@dsyme
Copy link
Contributor Author

dsyme commented Aug 1, 2020

So I think there's definitely room to improve the payload. 1MB isn't too bad I guess, but that is getting into big territory

Right. My concern is actually more what happens if we also have other community components in the same search index. But mind you FSHarp.Core is particularly large.

Let's see if it's an issue. Perhaps just compressing it would be enough

@cartermp
Copy link
Member

cartermp commented Aug 1, 2020

Re: re-downloading - a modern browser should cache it, at least I'm seeing that on firefox

@dsyme
Copy link
Contributor Author

dsyme commented Aug 1, 2020

a modern browser should cache it, at least I'm seeing that on firefox

good! Yes here it is loading from disk cache on Chrome:

image

@baronfel
Copy link
Collaborator

baronfel commented Aug 1, 2020

It's important that static resources like the search index be requested with a cache-busting query parameter, so that if the content changes the url parameter can be changed as well, causing a new fetch of the now-updated resource.

As things stand now, if we change index generation's schema without this additional parameter, previous visitors will be using the cached versions they have.

@dsyme
Copy link
Contributor Author

dsyme commented Aug 1, 2020

It's important that static resources like the search index be requested with a cache-busting query parameter, so that if the content changes the url parameter can be changed as well, causing a new fetch of the now-updated resource.

As things stand now, if we change index generation's schema without this additional parameter, previous visitors will be using the cached versions they have.

Cool. How do we check that? Here's the request: https://github.com/fsprojects/FSharp.Formatting/blob/master/docs/content/fsdocs-search.js#L14

@dsyme
Copy link
Contributor Author

dsyme commented Aug 1, 2020

@baronfel @cartermp Are you both ok with renaming this project to "docs"? (or maybe fsharp-docs since docs clashes with dotnet/docs and others?) Also is it possible to get docs.fsharp.org to redirect here?

@dsyme
Copy link
Contributor Author

dsyme commented Aug 1, 2020

Note if we want to reduce the size of index.json an easy way is to remove the remarks and exception sections from the search corpus

@dsyme
Copy link
Contributor Author

dsyme commented Aug 1, 2020

@baronfel I'll integrate this and let's iterate from here.

@dsyme dsyme merged commit 88c9645 into fsharp:master Aug 1, 2020
@Krzysztof-Cieslak
Copy link
Contributor

Not sure if my opinions are needed. It seems all the decisions have been already made here.

@dsyme
Copy link
Contributor Author

dsyme commented Aug 1, 2020

Not sure if my opinions are needed. It seems all the decisions have been already made here.

To be honest I really very much want your opinions, on all aspects....

I'm happy to change the entire technical basis of this effort if needed, as long as we keep things content driven (possibly apart from a template). All I care is that there is a tool we can run against the content we have (here FSharp.Core.dll and FSharp.Core.xml) and we generate usable quality docs.

@cartermp
Copy link
Member

cartermp commented Aug 1, 2020

Are you both ok with renaming this project to "docs"? (or maybe fsharp-docs since docs clashes with dotnet/docs and others?) Also is it possible to get docs.fsharp.org to redirect here?

No. Docs is a general term. Frankly I'm not sure what this repo does at this point other than run a tool in CI, but what it emits is very specific: the API reference for FSharp.Core. The repo should be named according to what it does.

fsharp/core-reference
fsharp/core-api-reference
fsharp/api-reference
fsharp/fsharp-core-reference
fsharp/fsharp-core-library-reference

etc.

docs.fsharp.org also doesn't point anywhere.

I think we need to slow down here and assess what it is we're actually trying to accomplish. Taking broad strokes to build and merge and completely change everything without time for feedback that's been asked for shouldn't be done. Is this:

  • An FSharp.Core API reference?
  • A more general API reference for other things in F#?
  • A general documentation site for F# that also includes FSharp.Core, effectively competing with the MS docs?
  • A stopgap for when FSharp.Core gets properly ingested into docs.microsoft.com?
  • A place for people to make meaningful contributions to the docs site? (see here: Adjust the logo link (was Contribution needs improving) #30)
  • An experiment to test FSharp.Formatting?

At this point I'm not clear about it anymore. The original goal was to act as a stopgap to eventually be subsumed by the MS docs site.

@baronfel
Copy link
Collaborator

baronfel commented Aug 1, 2020

echoing @cartermp's comments here, there have been a lot of changes done quite recently that, to be honest, feel like a regression in some ways.

As an example: the styling we had before was pretty streamlined and looked, if not completely customized for F# at least streamlined and modern. Now what's out and deployed looks spartan, in a not good way. @Krzysztof-Cieslak spent a lot of time finding an existing design system and making pages based off of that.

Similarly, I liked the older fashion of having namespace-level pages rather than a (potentially very long) scrollbar with all of the namespaces in the library as the top-level landing page.

I usually try to do OSS stuff on the weekend, so weekday stuff is either related to work (and so I can reasonably spend time on it during the workday), or quick, low-effort stuff like the search index fix here a couple days ago. It's hard for me to stay on top of the scope of the changes here, and it makes me want to invest that time less when changes are merged without time to trial and provide feedback.

I understand that this is an area of focus for you right now @dsyme, and love the changes to F#F overall, but it doesn't feel like I'm a contributor here so much as along for the ride ;)

Cool. How do we check that? Here's the request: fsprojects/FSharp.Formatting@master/docs/content/fsdocs-search.js#L14

Regarding this point, I'd suggest that that content rendered by the templating engine:

  • render the content, and
  • provide a hashed value for this specific content

Then, when rendering links/src elements to generated content files, append the hash to the query string. This would mean that urls like the one in the search JS you linked would need to be templated as well. Same for css links in the html head elements, etc.

@dsyme
Copy link
Contributor Author

dsyme commented Aug 1, 2020

As an example: the styling we had before was pretty streamlined and looked, if not completely customized for F# at least streamlined and modern.

Please redesign the core F#F template and/or CSS for it. But aim to get that into FSharp.Formatting, so all docs for all sites can make use of it.

...finding an existing design system and making pages based off of that.

Please describe what you have in mind in terms of design - examples to link to, principles, etc.

@dsyme
Copy link
Contributor Author

dsyme commented Aug 1, 2020

I think we need to slow down here and assess what it is we're actually trying to accomplish. Taking broad strokes to build and merge and completely change everything without time for feedback that's been asked for shouldn't be done.

I'll start a new thread to discuss the full range of F# documentation and tooling issues.

@dsyme dsyme mentioned this pull request Aug 1, 2020
@dsyme
Copy link
Contributor Author

dsyme commented Aug 1, 2020

See #32 for the discussion thread

For now I've renamed the repo to fsharp-core-docs and adjusted the inward links

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

Successfully merging this pull request may close these issues.

4 participants