Skip to content

Added warning for multiple root pages #1188

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

Added Warning for Multiple Root Pages Issue #1170

This pull request introduces warnings to alert developers when their documentation contains multiple root pages, resolving issue #1170. These warnings aim to highlight common configuration errors when using the docc convert tool directly, improving the debugging experience for documentation authors.

Checklist

  • [✅] Added tests to verify warning behavior across all scenarios.
  • [✅] Ran the ./bin/test script and confirmed it succeeded.

@ayushshrivastv ayushshrivastv changed the title Added warning for multiple root pages (#1170) Added warning for multiple root pages Apr 1, 2025
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.

This is your 4th PR for the same issue and you haven't addressed the feedback from the 3rd PR. If you open a 5th PR I will immediately close it without reviewing.

When you need to make changes to a PR, do so by updating/editing the existing PR. Don't open a new one for each change. If you don't know how to update a PR, ask for help or do a web search for "github update pull request" and see if some of the search results tell you how.

@ayushshrivastv
Copy link
Author

ayushshrivastv commented Apr 1, 2025

I’ve been testing the code locally, and it seems to be working fine. I’ve tried different scenarios, like creating multiple symbol graphs and pages, and the code passes all the tests. But the warning message is still there, and I can’t figure out why file it’s not formatted correctly. I’ve tried switching to a different branch and unchecking all the formatting settings, but the problem keeps coming back. When I push the code again, I get the same error.

To fix this, I tried pushing the code with different branches, hoping that would help. But that didn’t work either. Now I’m trying to figure out what’s wrong by looking at the Swift forums. I know I made a mistake, and I’m really sorry for any trouble this has caused.

The ./bin/test script runs flawlessly without any errors related to my changes. But it exited with a code 1 because of a license header check failure. It couldn’t find the required headers in the file `‘./bin/make-test-bundle/.build/arm64-apple-macosx/debug/make_test_bundle.build/DerivedSources/resource_bundle_accessor.swift’.

@franklinsch
Copy link
Contributor

Hi @ayushshrivastv, thank you for your interest in contributing! As David pointed out above, it's preferred to keep the same PR to iterate on changes so that we can keep the feedback centralized. It seems like there is still feedback to address from the previous PRs, so let us know when your work is ready for review.

For the formatting issues, you can use git checkout to restore the state of the file to a previous version. Let us know if we can provide guidance, and thank you for contributing!

@ayushshrivastv
Copy link
Author

ayushshrivastv commented Apr 1, 2025

Thank you for your help.

What’s happening is after adding the code to support multiple root pages and saving the DocumentationContext.swift file, everything appears to be in order. However, once I commit the file, a significant number of lines are unexpectedly reformatted, and I haven’t been able to determine the cause.

I recently merged two commits into the OpenAPIKit library without any problems, so this appears to be unrelated. I’ve reviewed discussions on the Swift forums but haven’t found a solution that addresses this behavior. I’ll update the commit as soon as the issue is resolved.

mattpolzin/OpenAPIKit#404
mattpolzin/OpenAPIKit#405

@d-ronnqvist
Copy link
Contributor

This repository doesn't have a formatter. My guess is that something in your local workflow is trimming whitespace lines in the entire file, not just the lines that you changed.

You can either use git diff --cached to see the staged changes before you commit or git show to see the changes of the commit before you push.

@ayushshrivastv
Copy link
Author

@d-ronnqvist In response to your feedback about whitespace, I’ve made some changes.

a8d4721

Thank You!

@d-ronnqvist
Copy link
Contributor

@swift-ci please test

@d-ronnqvist
Copy link
Contributor

@ayushshrivastv This PR still contains all the whitespace changes

}

XCTAssertEqual(multipleRootModulesWarnings.count, 1)
XCTAssertEqual(multipleRootModulesWarnings[0].diagnostic.summary, "Documentation contains symbol graphs for multiple main modules: Module1, Module2")
Copy link
Contributor

@d-ronnqvist d-ronnqvist Apr 17, 2025

Choose a reason for hiding this comment

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

This test is not reliable because the order that the modules (and articles) are processed isn't stable. The CI failure confirms this:

XCTAssertEqual failed: ("Documentation contains symbol graphs for multiple main modules: Module2, Module1") is not equal to ("Documentation contains symbol graphs for multiple main modules: Module1, Module2")

@ayushshrivastv
Copy link
Author

ayushshrivastv commented Apr 17, 2025

Thank you for your feedback! @d-ronnqvist

At the moment, I’m manually cleaning up excessive whitespace in the files. Unfortunately, each time I make changes, a significant amount of whitespace tends to get introduced, which makes debugging more difficult. I understand this can be frustrating, and I sincerely apologize for the inconvenience.

The good news is that I’ve identified the core issue—it appears to be related to my development environment. I was working in VS Code, where the problem persisted, but after switching to Xcode, I’m no longer experiencing the same issue. With that resolved, I can now focus on fixing the remaining problems in the repository.

In parallel, I’ve begun exploring Swift DocC and the OpenAPIKit library to deepen my understanding of the codebase. I’m currently developing a tool that aims to bridge OpenAPI specifications with Swift’s DocC documentation
framework, making it possible to automatically generate DocC documentation directly from OpenAPI files.

OpenAPI-Integration-with-DocC

@ayushshrivastv ayushshrivastv force-pushed the Added-warnings-for-multiple-root-pages-1170 branch from 248da00 to 556736e Compare April 18, 2025 09:12
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