-
Notifications
You must be signed in to change notification settings - Fork 28
PRPr 111 #112
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
PRPr 111 #112
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds comprehensive GitHub Pages documentation infrastructure and CloudKit/CloudDrive storage provider test coverage for the ManagedCode.Storage project. The changes include Jekyll-based documentation site setup, feature/API/ADR documentation, SEO optimization, and HTTP-handler-based test fakes for CloudKit, OneDrive, Google Drive, and Dropbox providers.
Key Changes
- GitHub Pages documentation site with custom layout, CSS, SEO metadata, sitemap, and robots.txt
- Documentation for all storage providers (Azure, AWS, GCS, FileSystem, SFTP, OneDrive, Google Drive, Dropbox, CloudKit), features (VFS, chunked uploads, DI), and integrations (ASP.NET Server, HTTP/SignalR clients)
- CloudKit storage provider tests with fake HTTP handler
- CloudDrive provider tests (OneDrive/Graph, Google Drive, Dropbox) using HTTP message handlers wired into official SDK clients
- SEO audit document and development setup guide
Reviewed changes
Copilot reviewed 104 out of 161 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| github-pages/_layouts/default.html | Jekyll layout with SEO metadata, structured data, navigation, dark mode toggle |
| github-pages/_config.yml | Site configuration with SEO keywords and Jekyll settings |
| github-pages/sitemap.xml | XML sitemap for search engines |
| github-pages/robots.txt | Robots directives with sitemap reference |
| github-pages/assets/css/style.css | Custom CSS with dark mode support and responsive design |
| github-pages/*.md | Page templates for documentation site (home, setup, testing, templates, etc.) |
| docs/Features/*.md | Feature documentation for providers and integrations |
| docs/API/*.md | HTTP and SignalR API documentation |
| docs/ADR/*.md | Architecture Decision Records |
| docs/Development/*.md | Setup guide, credentials guide, SEO audit |
| docs/Testing/strategy.md | Test strategy and suite structure |
| docs/templates/*.md | Feature and ADR templates |
| docs/server-streaming-plan.md | Added Mermaid diagram for architecture |
| Tests/.../CloudKit/*.cs | CloudKit storage tests with fake HTTP handler |
| Tests/.../CloudDrive/*.cs | OneDrive/Google Drive/Dropbox tests with HTTP fakes |
| Tests/.../GCS/GCSUploadTests.cs | Discard unused parameter to suppress warning |
| Tests/.../FileSystem/FileSystemUploadTests.cs | Code formatting (spacing around operators) |
| dotnet-install.sh | .NET SDK installation script (Microsoft official) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| foreach (var part in query.TrimStart('?').Split('&', StringSplitOptions.RemoveEmptyEntries)) | ||
| { | ||
| var kv = part.Split('=', 2); | ||
| var key = Uri.UnescapeDataString(kv[0]); | ||
| var value = kv.Length == 2 ? Uri.UnescapeDataString(kv[1]) : string.Empty; | ||
| result[key] = value; | ||
| } |
Copilot
AI
Dec 15, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This foreach loop immediately maps its iteration variable to another variable - consider mapping the sequence explicitly using '.Select(...)'.
| foreach (var part in query.TrimStart('?').Split('&', StringSplitOptions.RemoveEmptyEntries)) | ||
| { | ||
| var kv = part.Split('=', 2); | ||
| var key = Uri.UnescapeDataString(kv[0]); | ||
| var value = kv.Length == 2 ? Uri.UnescapeDataString(kv[1]) : string.Empty; | ||
| result[key] = value; | ||
| } |
Copilot
AI
Dec 15, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This foreach loop immediately maps its iteration variable to another variable - consider mapping the sequence explicitly using '.Select(...)'.
| } | ||
|
|
||
| _entries.Remove(entry.Id); | ||
| response = new HttpResponseMessage(HttpStatusCode.NoContent); |
Copilot
AI
Dec 15, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Disposable 'HttpResponseMessage' is created but not disposed.
| response = new HttpResponseMessage(HttpStatusCode.OK) | ||
| { | ||
| Content = new ByteArrayContent(existing.Content) | ||
| }; |
Copilot
AI
Dec 15, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Disposable 'HttpResponseMessage' is created but not disposed.
|
|
||
| private bool TryHandleChildrenRequest(HttpRequestMessage request, out HttpResponseMessage response) | ||
| { | ||
| response = new HttpResponseMessage(HttpStatusCode.NotFound); |
Copilot
AI
Dec 15, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Disposable 'HttpResponseMessage' is created but not disposed.
|
|
||
| private bool TryHandleItemRequest(HttpRequestMessage request, out HttpResponseMessage response) | ||
| { | ||
| response = new HttpResponseMessage(HttpStatusCode.NotFound); |
Copilot
AI
Dec 15, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Disposable 'HttpResponseMessage' is created but not disposed.
| // Arrange | ||
|
|
||
| var uploadStream1 = new MemoryStream(90*1024); | ||
| var uploadStream1 = new MemoryStream(90 * 1024); |
Copilot
AI
Dec 15, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Disposable 'MemoryStream' is created but not disposed.
No description provided.