Skip to content

Conversation

cjsha
Copy link
Member

@cjsha cjsha commented Aug 22, 2025

fix #262

@cjsha cjsha requested a review from bparks13 August 22, 2025 02:43
@bparks13
Copy link
Member

@cjsha One concern I initially had when checking the release notes is this PR, which introduces a breaking change on a minor version release (which seems strange to me). However, looking at it more, it seems that even though the source library is built on .NET 4.7.2, this has no effect on the build process, since we are specifying .NET 8 as the target framework for DocFx, and the documentation build is separate from compiling the library.

The fact that the checks passed here also mean that the build process seems unaffected. However, I still want to step through the docs and make sure there aren't any unexpected changes on any pages before we merge this.

Copy link
Member

@bparks13 bparks13 left a comment

Choose a reason for hiding this comment

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

Tested this locally and stepped through a number of pages, nothing seems to be out of place. Images / SVGs work as expected, copying/pasting workflows works, videos play correctly.

One note, in testing the search function, for ContextTask specifically, it will correctly find the page if I only type context, but as soon as I add more it shows 0 results. However, if i write the whole thing contexttask it then pops up again. Reading the PR / release notes this makes sense, as the search function is not really testing the whole substring, but rather testing specific combinations of tokens taken from the names. So, this behavior is expected, even if it is a little annoying.

@cjsha
Copy link
Member Author

cjsha commented Aug 22, 2025

yeah the search is still finicky in the way you described. I don't think this improves search in the way I was hoping it would. I think it only really helps in the context of searches with multiple terms. With this PR, additional terms narrow down the search instead of broadening the scope of the search.

I also still think it's better than ContextTask not showing up until the 3rd page when "context" is searched. I think, with this update, articles with search term will show up in the search results first.

@cjsha cjsha merged commit cf10c80 into main Aug 22, 2025
3 checks passed
@cjsha cjsha deleted the issue-262 branch August 22, 2025 22:59
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.

Update docfx to latest version
2 participants