Skip to content

Added warnings for multiple root pages #1187

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

ayushshrivastv
Copy link

@ayushshrivastv ayushshrivastv commented Apr 1, 2025

Summary

The implementation in DocumentationContext.swift is correctly handling all three warning scenarios for multiple root pages:

Multiple symbol graph modules - Detected when multiple symbol graph files are included, with a warning identifier org.swift.docc.MultipleSymbolGraphRoots

Mixed root types - Detected when both symbol graph modules and manual technology roots are present, with warning identifier org.swift.docc.MixedRootTypes

Multiple manual technology roots - Detected when multiple pages with @TechnologyRoot directive exist, with warning identifier org.swift.docc.MultipleTechnologyRoots

Testing

All tests for the DocumentationContext_RootPageTests have passed successfully!
swift test --filter DocumentationContext_RootPageTests

This test verifies that the system correctly identifies and warns when documentation contains symbol graphs for multiple modules.
testMultipleSymbolGraphModulesWarning - PASSED

This test confirms that warnings are correctly emitted when documentation contains both a symbol graph module and a manual technology root article with the @TechnologyRoot directive.
testMixedRootTypesWarning - PASSED

This test validates that the system properly identifies and warns when documentation contains multiple manual technology roots.
testMultipleTechnologyRootsWarning - PASSED

Screenshot 2025-04-01 at 10 18 43 AM

Screenshot 2025-04-01 at 10 19 18 AM

Steps:
swift test --filter DocumentationContext_RootPageTests.testMultipleSymbolGraphModulesWarning

swift test --filter DocumentationContext_RootPageTests.testMixedRootTypesWarning

swift test --filter DocumentationContext_RootPageTests.testMultipleTechnologyRootsWarning

Checklist

  • [✅] Added tests
  • [✅ ] Ran the ./bin/test script and it succeeded
  • Updated documentation if necessary

It would be greatly appreciated if you could review the code once @d-ronnqvist . I have attempted to implement the previous feedback and made some local modifications. Please inform me if there are any suggestions or if the code is not functioning as intended. This is my first GitHub contribution to an organization repository, and I’m eager to learn from any mistakes.

Copy link
Contributor

@d-ronnqvist d-ronnqvist left a comment

Choose a reason for hiding this comment

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

Hi. Like your first PR, this reformats a large amount of code that's otherwise unmodified. It's less than the original PR but still a significant amount of format-only changes.

Before someone can review this, please remove these format-only changes so that the PR only contains your new code.

@d-ronnqvist
Copy link
Contributor

To make it easier for people to review this PR, please try to describe the user experience of these changes and the how a reviewer can try it out that's more inline with the instructions of the PR template. It's not relevant for a reviewer to run the tests. That doesn't let them experience the changes. Instead, a reviewer wants to know what content they need to add to encounter the new warning and what information the warning should contain. This high level information can help you and the reviewer to talk about the user experience without associating the feedback to a specific file or line of code.

@ayushshrivastv
Copy link
Author

ayushshrivastv commented Apr 1, 2025

I don’t know why my files keep getting reformatted when I push commits to this repository, even though I’ve turned off all formatter settings and see no errors during the commit process. The issue persists, but I hope it will stop eventually, though I’m unsure of the reason behind it. I have raised another PR. I will work on formatting the lines between the code. It is working properly, and I have tested it locally!

Sorry for any issue caused.

@ayushshrivastv ayushshrivastv deleted the Added-warning-for-multiple-root-pages-Issue-1170 branch April 1, 2025 11:25
@Kyle-Ye
Copy link
Contributor

Kyle-Ye commented Apr 18, 2025

  1. You can always check/pick your changes line by line by ANY Git tool. And only commit the desired lines.

  2. I guess you can check your Xcode editor setting here to avoid such issue.

image

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.

3 participants