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: broken documentation indexing with correct URLs #615

Merged
merged 4 commits into from
Oct 1, 2024

Conversation

galuszkak
Copy link
Contributor

This PR fixes indexing that has been broken by #550 .

Also enables indexing of Container level decisions and documentation. To be fully consistent with new structure.

@galuszkak galuszkak changed the title fix: Fix broken Indexing with correct URLs fix: broken documentation indexing with correct URLs Sep 27, 2024
Copy link
Collaborator

@jenspav jenspav left a comment

Choose a reason for hiding this comment

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

Thanks a lot for addressing this! Looks good, just need a few more tests.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Would you mind adding tests for no container sections, no container decisions, indexes container decisions and indexes container sections, similar like the existing ones for software systems?

While at it, no software system documentation should be named no container sections.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jenspav added, please review.

I didn't quite catch the last comment, this is actually testing no software system documentation and has nothing to do with containers, so I'm unsure what you referring there.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I didn't quite catch the last comment, this is actually testing no software system documentation and has nothing to do with containers, so I'm unsure what you referring there.

Sry, typo on my side, what I meant was: Could you please also rename this function https://github.com/avisi-cloud/structurizr-site-generatr/blob/main/src/test/kotlin/nl/avisi/structurizr/site/generatr/site/model/IndexingTest.kt#L282 to no software system section for consistency with the other tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jenspav done, please review now.

)
}/sections/${section.order}".asUrlToDirectory(viewModel.url),
"Documentation",

Copy link
Collaborator

@jenspav jenspav Sep 30, 2024

Choose a reason for hiding this comment

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

Sorry, missed this one earlier, could you please remove this empty line? Everything else LGTM!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jenspav done!

@jenspav jenspav merged commit f947984 into avisi-cloud:main Oct 1, 2024
2 checks passed
@jenspav
Copy link
Collaborator

jenspav commented Oct 1, 2024

Thanks!

@galuszkak galuszkak deleted the fix/search-links branch October 1, 2024 08:34
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.

2 participants