Fix critical issues found in code review#18
Open
coder2000 wants to merge 8 commits into
Open
Conversation
- Updated `.gitignore` to include new entries for Visual Studio, Visual Studio Code, and various build outputs, while removing outdated entries. - Added rules for ignoring temporary files, build artifacts, and user-specific settings. - Added package references for `FluentAssertions`, `Microsoft.NET.Test.Sdk`, and `xunit.runner.visualstudio` in `Ubiety.Xmpp.Test.csproj` to improve testing capabilities.
Updated `appveyor-old.yml` to include a new configuration with secure environment variables and PowerShell scripts, while disabling test and deploy steps. Modified `appveyor.yml` to use Visual Studio 2022, added auto-generated code instructions, and replaced environment variables with new build commands for Windows and Unix-like systems. Integrated AppVeyor CI in `Build.cs` by adding the `Nuke.Common.CI.AppVeyor` namespace and an `AppVeyor` attribute for the `Build` class. Set `FetchDepth` to 0 in GitHub Actions to fetch full history.
Removed unnecessary using directives and TypeConverter attribute from Configuration.cs. This change simplifies the class by eliminating unused namespaces and the type conversion functionality, reflecting a shift in how the class is utilized.
Reorganized namespaces from `Ubiety.Xmpp.Core` to `Ubiety.Xmpp.Core.Client` for better structure. Added new `IServer` interface and `XmppServer` class to introduce server-side functionality. Updated `Execute` method signatures in state classes to enforce non-nullable `Tag` parameters. Added necessary using directives across multiple files. Removed unused `using System;` directive in `XmppBuilder.cs`. Minor copyright comment formatting adjustments.
- Parser: add missing _cts and _disposed field declarations that were used but never declared, causing a compile error - AsyncClientSocket: store BeginReadAsync task in _readTask field and wrap read loop in try/catch so exceptions are logged rather than silently swallowed - BindingState: complete resource binding by appending the <resource> element when a client resource is set, and handle the server IQ result/error response instead of leaving the else branch empty Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
GitVersion 6.x (referenced via PackageDownload in _build.csproj) requires an explicit workflow configuration. Without GitVersion.yml it cannot determine the versioning strategy, causing build failures. - Add GitVersion.yml with GitFlow/v1 workflow; override the main branch regex to match both 'main' and 'master' since the repo uses both - Remove the leftover GitVersion.CommandLine.DotNetCore 5.0.0 nupkg from packages/ which is unused now that the build uses GitVersion.Tool 6.4.0 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The generated continuous.yml was missing fetch-depth: 0 on both checkout steps, so GitHub Actions checked out a shallow clone (depth 1). GitVersion requires full history to calculate semantic versions. FetchDepth = 0 was already set on the [GitHubActions] attribute in Build.cs but the workflow file was not regenerated after that change. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude Code tooling files are local to the developer's environment and should not be tracked in the repository. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
_ctsand_disposedfield declarations —both were referenced throughout the class but never declared, causing
a compile error
BeginReadAsyncresult in a_readTaskfield and wrap the read loop in a try/catch so exceptions are logged
rather than silently swallowed as unobserved task exceptions
the
<resource>element when a client resource is configured, and handlethe server's IQ result/error response (the else branch was previously
empty, meaning binding could never complete successfully)
Test plan