Upgrade to .NET 10 #144
Open
Upgrade to .NET 10 #144
Conversation
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
There was a problem hiding this comment.
Pull request overview
Upgrades the build and project configuration to target .NET 10 and updates the repo’s solution/build setup accordingly.
Changes:
- Switches the repo from a
.slnto a new XML-based.slnxsolution file and updatesbuild.ps1to use it. - Updates all projects to target
net10.0and enables nullable + implicit usings with C# language version pinned. - Updates CI to build with the .NET 10 SDK and refreshes test package dependencies; adds an additional
.gitignoreentry.
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| build.ps1 | Points the build pipeline at the new Solutions/Argotic.slnx solution file. |
| Solutions/Argotic.slnx | New solution definition listing the projects to build. |
| Solutions/Argotic.sln | Removes the old Visual Studio .sln solution file. |
| Solutions/Argotic.Common/Argotic.Common.csproj | Moves the library to net10.0 and enables nullable/implicit usings (breaking change vs prior multi-targeting). |
| Solutions/Argotic.Core/Argotic.Core.csproj | Moves to net10.0, enables nullable/implicit usings, and bumps a dependency version. |
| Solutions/Argotic.Extensions/Argotic.Extensions.csproj | Moves to net10.0 and enables nullable/implicit usings (breaking change vs prior multi-targeting). |
| Solutions/Argotic.Examples/Argotic.Examples.csproj | Moves to net10.0 and enables nullable/implicit usings (breaking change vs prior multi-targeting). |
| Solutions/Argotic.Extensions.Tests/Argotic.Extensions.Tests.csproj | Moves tests to net10.0 and updates test-related package versions. |
| .gitignore | Adds .endjin/ to ignored paths. |
| .github/workflows/build.yml | Updates CI to install/use .NET SDK 10.x only. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| <PropertyGroup> | ||
| <TargetFrameworks>netstandard2.0;netstandard2.1;net8.0;net9.0;</TargetFrameworks> | ||
| <TargetFramework>net10.0</TargetFramework> |
There was a problem hiding this comment.
Changing from multi-targeting (TargetFrameworks) to a single TargetFramework (net10.0) drops support for netstandard2.0/2.1 and net8/net9 consumers. If Argotic.Core is intended to remain broadly consumable, consider multi-targeting and adding net10.0 instead (or explicitly treat this as a breaking change with appropriate versioning/docs).
Suggested change
| <TargetFramework>net10.0</TargetFramework> | |
| <TargetFrameworks>netstandard2.0;netstandard2.1;net8.0;net9.0;net10.0</TargetFrameworks> |
|
|
||
| <PropertyGroup> | ||
| <TargetFrameworks>netstandard2.1;netstandard2.0;net8.0;net9.0;</TargetFrameworks> | ||
| <TargetFramework>net10.0</TargetFramework> |
There was a problem hiding this comment.
Switching from TargetFrameworks to a single net10.0 TargetFramework removes previously supported TFMs (netstandard2.x, net8, net9). If Argotic.Extensions is distributed as a library package, consider keeping multi-targeting and adding net10.0, or make the breaking-change intent explicit (including package versioning).
Suggested change
| <TargetFramework>net10.0</TargetFramework> | |
| <TargetFrameworks>netstandard2.0;net8.0;net9.0;net10.0</TargetFrameworks> |
|
|
||
| <PropertyGroup> | ||
| <TargetFrameworks>netstandard2.0;netstandard2.1;net8.0;net9.0;</TargetFrameworks> | ||
| <TargetFramework>net10.0</TargetFramework> |
There was a problem hiding this comment.
Switching Argotic.Examples from TargetFrameworks to a single net10.0 TargetFramework drops prior TFMs (netstandard2.x/net8/net9). If this project needs to remain buildable/runable on older frameworks or be referenced by them, consider restoring multi-targeting and adding net10.0.
Suggested change
| <TargetFramework>net10.0</TargetFramework> | |
| <TargetFrameworks>netstandard2.0;net8.0;net9.0;net10.0</TargetFrameworks> |
|
|
||
| <PropertyGroup> | ||
| <TargetFrameworks>netstandard2.0;netstandard2.1;net8.0;net9.0;</TargetFrameworks> | ||
| <TargetFramework>net10.0</TargetFramework> |
There was a problem hiding this comment.
Changing from multi-targeting (TargetFrameworks) to a single TargetFramework (net10.0) drops support for netstandard2.0/2.1 and net8/net9 consumers. If these projects are published as reusable libraries, consider keeping TargetFrameworks and adding net10.0 (or otherwise document this breaking change and ensure versioning reflects it).
Suggested change
| <TargetFramework>net10.0</TargetFramework> | |
| <TargetFrameworks>netstandard2.0;netstandard2.1;net8.0;net9.0;net10.0</TargetFrameworks> |
Get-EndjinGist zf prautoflow Get-EndjinGist zf dotnet
Test Results3 697 tests +3 583 3 697 ✅ +3 603 7s ⏱️ +7s Results for commit d3382d1. ± Comparison against base commit 37fc3cf. This pull request removes 114 and adds 3697 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
…cross multiple classes for consistency and adherence to C# coding conventions. Updated comparison methods to use lowercase 'string' as well.
…bility in SyndicationDateTimeUtility and BasicGeocodingSyndicationExtension
… utility classes and tests
Migrate from the custom Guard class to modern .NET 10 built-in exception helpers, which use CallerArgumentExpression to automatically capture parameter names without manual string arguments. Replacements: - Guard.ArgumentNotNull -> ArgumentNullException.ThrowIfNull (1,322 calls) - Guard.ArgumentNotNullOrEmptyString -> ArgumentException.ThrowIfNullOrEmpty (140 calls) - Guard.ArgumentNotGreaterThan -> ArgumentOutOfRangeException.ThrowIfGreaterThan (7 calls) - Guard.ArgumentNotLessThan -> ArgumentOutOfRangeException.ThrowIfLessThan (13 calls) Co-Authored-By: Claude Opus 4.5 <[email protected]>
…eractively or non-interactively a) to show how to use the APIs b) to prove that the APIs work.
- Fix RFC 822 date parsing for numeric timezone offsets (+00:00, -05:00)
in SyndicationDateTimeUtility.ReplaceRfc822TimeZoneWithOffset
- Fix XPath expression bug in TrackbackDiscoveryMetadata.Load (\\r -> /)
New test coverage for:
- Legacy RSS adapters (0.90, 0.91, 0.92, 1.0)
- Legacy Atom 0.3 and RSD 0.6 adapters
- BlogML attachment and trackback types
- XML-RPC array and structure member types
- Discovery types (TrackbackDiscoveryMetadata, DiscoverableSyndicationEndpoint)
- MimeMediaTypeAttribute
- Configuration/DI (ServiceCollectionExtensions, ClientOptions)
- Async utility methods (SyndicationDiscoveryUtility)
Coverage improved from 56.2% to 66.1% line coverage (+10%)
Implement complete support for the Sitemap 0.9 specification, enabling
web content syndication for search engine crawlers.
Core Classes:
- Sitemap: Main class implementing ISyndicationResource with async/sync
loading from Uri, Stream, XmlReader, and IXPathNavigable
- SitemapUrl: Individual URL entry with Location, LastModified,
ChangeFrequency, and Priority (0.0-1.0) properties
- SitemapIndex: Container for multiple sitemaps (up to 50,000 entries)
- SitemapIndexEntry: Individual sitemap reference with Location and
LastModified
- SitemapChangeFrequency: Enum (Always, Hourly, Daily, Weekly, Monthly,
Yearly, Never)
- SitemapUtility: Helper methods for parsing and serialization
Extensions (Google Sitemap extensions):
- SitemapImageExtension: Image metadata (caption, geo-location, license)
- SitemapVideoExtension: Video metadata (duration, rating, view count,
platform targeting, player location)
- SitemapNewsExtension: News article metadata (publication, dates)
- SitemapHreflangExtension: Alternate language versions for i18n SEO
Infrastructure:
- Sitemap09SyndicationResourceAdapter for XML parsing
- Full extension support via IExtensibleSyndicationObject
Testing:
- Comprehensive test suite (~3,300 LOC) covering parsing, serialization,
utility methods, and all extensions
Examples:
- SitemapExample: Creating and loading sitemaps
- SitemapIndexExample: Managing sitemap indexes
- Sample XML files for sitemap and sitemap index
Implement complete video:id and content_segment_loc element support per
the Google Video Sitemap 1.1 XSD specification, along with examples and
tests for all sitemap extensions.
New classes:
- SitemapVideoIdType enum for video identifier types
- SitemapVideoId class for video:id element support
- SitemapVideoSegment class for content_segment_loc support
SitemapVideo enhancements:
- Add MaxTitleLength (100 chars) constant with truncation
- Add Identifiers and ContentSegments collections
- Add deprecation notice for elements removed by Google in May 2022
Documentation:
- Add XSD schema references to SitemapVideo, SitemapVideoExtension,
SitemapImage, SitemapImageExtension, SitemapNewsExtension,
and SitemapNewsPublication classes
Examples:
- Add SitemapVideoExtensionExample with 8 usage examples
- Add SitemapImageExtensionExample with 6 usage examples
- Add SitemapNewsExtensionExample with 7 usage examples
- Add sample XML files: sitemap_video.xml, sitemap_image.xml,
sitemap_news.xml
- Update SampleDataPath and ExampleOutput helpers
Tests:
- Add SitemapVideoTests with 58 comprehensive unit tests
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 75 out of 428 changed files in this pull request and generated 11 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /// A <see cref="IList{T}"/> collection of <see cref="XmlRpcStructureMember"/> objects that represent this structure's members. | ||
| /// The default value is an <i>empty</i> collection. | ||
| /// </value> | ||
| public IList<XmlRpcStructureMember> Members { get; } = []; |
There was a problem hiding this comment.
Initializing an IList<T> with [] is risky because it may produce a T[] at runtime (arrays implement IList<T> but are fixed-size). This class later calls Members.Add(...), which would throw NotSupportedException if Members is backed by an array. Prefer initializing with a growable list (e.g., new List<XmlRpcStructureMember>() or keep Collection<XmlRpcStructureMember>).
Suggested change
| public IList<XmlRpcStructureMember> Members { get; } = []; | |
| public IList<XmlRpcStructureMember> Members { get; } = new List<XmlRpcStructureMember>(); |
| /// A <see cref="IList{T}"/> collection of <see cref="IXmlRpcValue"/> objects that represent the data elements for this array. | ||
| /// The default value is an <i>empty</i> collection. | ||
| /// </value> | ||
| public IList<IXmlRpcValue> Values { get; } = []; |
There was a problem hiding this comment.
Same issue as above: [] assigned to IList<IXmlRpcValue> may produce an array-backed list. Since this type calls Values.Add(...), it can throw NotSupportedException at runtime. Initialize with a mutable collection (e.g., new List<IXmlRpcValue>()).
Comment on lines
+7
to
+11
| public string HRef { get; set; } | ||
|
|
||
| public Dictionary<string, string> Attributes { get; set; } | ||
| public Dictionary<string, string> Attributes { get; } = []; | ||
|
|
||
| public string Title { get; set; } | ||
| } | ||
| public string Title { get; set; } |
There was a problem hiding this comment.
With <Nullable>enable</Nullable>, HRef and Title are non-nullable but not initialized, which will generate nullable warnings (and may become errors depending on repo settings). Consider initializing them (e.g., = string.Empty;) or making them nullable (string?) if they are optional.
| /// This indexer uses a <i>case-insensitive</i> comparison of the specified member <paramref name="name"/>. | ||
| /// </remarks> | ||
| /// <exception cref="ArgumentNullException">The <paramref name="name"/> is a null reference.</exception> | ||
| /// <exception cref="ArgumentNullException">The <paramref name="name"/> is an empty string.</exception> |
There was a problem hiding this comment.
ArgumentException.ThrowIfNullOrEmpty(name) throws ArgumentNullException for null and ArgumentException for empty. The XML docs currently claim an ArgumentNullException for empty strings, which is no longer accurate. Update the <exception> documentation (or change the guard to match the documented exception types).
Suggested change
| /// <exception cref="ArgumentNullException">The <paramref name="name"/> is an empty string.</exception> | |
| /// <exception cref="ArgumentException">The <paramref name="name"/> is an empty string.</exception> |
| get | ||
| { | ||
| Guard.ArgumentNotNullOrEmptyString(name, "name"); | ||
| ArgumentException.ThrowIfNullOrEmpty(name); |
There was a problem hiding this comment.
ArgumentException.ThrowIfNullOrEmpty(name) throws ArgumentNullException for null and ArgumentException for empty. The XML docs currently claim an ArgumentNullException for empty strings, which is no longer accurate. Update the <exception> documentation (or change the guard to match the documented exception types).
Comment on lines
+16
to
+21
| directories: | ||
| - "/" | ||
| - "actions/**/*" | ||
| schedule: | ||
| interval: "daily" | ||
| open-pull-requests-limit: 10 |
There was a problem hiding this comment.
Dependabot config uses directory (singular), not directories. To monitor multiple directories you generally need multiple update entries (one per directory). As written, this block is likely invalid and may cause Dependabot to ignore the configuration.
Suggested change
| directories: | |
| - "/" | |
| - "actions/**/*" | |
| schedule: | |
| interval: "daily" | |
| open-pull-requests-limit: 10 | |
| directory: "/" | |
| schedule: | |
| interval: "daily" | |
| open-pull-requests-limit: 10 | |
| - package-ecosystem: "github-actions" | |
| directory: "/actions" | |
| schedule: | |
| interval: "daily" | |
| open-pull-requests-limit: 10 |
Comment on lines
22
to
28
| <ItemGroup> | ||
| <Compile Remove="Configuration\Provider\XmlSyndicationResourceProvider.cs" /> | ||
| <Compile Remove="Configuration\Provider\SyndicationResourceProvider.cs" /> | ||
| <Compile Remove="Configuration\Provider\SyndicationResourceProviderCollection.cs" /> | ||
| <Compile Remove="Syndication\SyndicationManager.cs" /> | ||
| </ItemGroup> | ||
|
|
There was a problem hiding this comment.
This project now explicitly excludes several provider types from compilation, but those files are still present (and updated in this PR). If the provider model is being removed, consider deleting the excluded source files (or moving them to a separate package/TFM) to avoid confusion and prevent other code from accidentally referencing non-compiled types.
Suggested change
| <ItemGroup> | |
| <Compile Remove="Configuration\Provider\XmlSyndicationResourceProvider.cs" /> | |
| <Compile Remove="Configuration\Provider\SyndicationResourceProvider.cs" /> | |
| <Compile Remove="Configuration\Provider\SyndicationResourceProviderCollection.cs" /> | |
| <Compile Remove="Syndication\SyndicationManager.cs" /> | |
| </ItemGroup> |
Comment on lines
+182
to
+185
| public Task<XPathNavigator> CreateNavigatorAsync(CancellationToken cancellationToken = default) | ||
| { | ||
| return CreateNavigatorAsync(SyndicationEncodingUtility.SharedHttpClient, cancellationToken); | ||
| } |
There was a problem hiding this comment.
It looks like the previous synchronous CreateNavigator() API was replaced with CreateNavigatorAsync(...). If this is a public API, that’s a breaking change for consumers. Consider keeping a synchronous overload for compatibility (possibly implemented as a thin wrapper over the async path) or providing a clear migration path in the API surface.
Comment on lines
+22
to
+35
| internal ConditionalGetResult(HttpResponseMessage? response, bool wasModified) | ||
| { | ||
| this.response = response; | ||
| WasModified = wasModified; | ||
|
|
||
| if (response != null) | ||
| { | ||
| StatusCode = response.StatusCode; | ||
| LastModified = response.Content.Headers.LastModified; | ||
| ETag = response.Headers.ETag?.Tag; | ||
| ContentLength = response.Content.Headers.ContentLength ?? -1; | ||
| ContentType = response.Content.Headers.ContentType?.MediaType; | ||
| } | ||
| } |
There was a problem hiding this comment.
When response is null, StatusCode remains the default value (0), which is not a valid HttpStatusCode and may confuse callers. Consider either making StatusCode nullable, or explicitly setting it to HttpStatusCode.NotModified (or another meaningful value) when WasModified == false and response is null.
| /// <exception cref="ArgumentNullException">The <paramref name="source"/> is a null reference (Nothing in Visual Basic).</exception> | ||
| bool Load(XPathNavigator source); | ||
| /// <param name="source">The <see cref="XPathNavigator"/> to extract information from.</param> | ||
| /// <returns><b>true</b> if the <see cref="IXmlRpcValue"/> was initialized using the supplied <paramref name="source"/>, Otherwise, <b>false</b>.</returns> |
There was a problem hiding this comment.
In this sentence, 'Otherwise' should be lowercase ('otherwise') for correct grammar.
Suggested change
| /// <returns><b>true</b> if the <see cref="IXmlRpcValue"/> was initialized using the supplied <paramref name="source"/>, Otherwise, <b>false</b>.</returns> | |
| /// <returns><b>true</b> if the <see cref="IXmlRpcValue"/> was initialized using the supplied <paramref name="source"/>, otherwise, <b>false</b>.</returns> |
BREAKING CHANGE: Remove non-generic CompareTo(object?) method
This is a breaking change that modernizes the comparison interface
implementation by replacing the legacy IComparable interface with
the strongly-typed IComparable<T> generic interface.
Changes:
- Update 8 types in Argotic.Common
- Update 54 types in Argotic.Core (Atom, RSS, OPML, Sitemap,
Publishing, APML, BlogML, RSD, Net, Generic)
- Update 57 types in Argotic.Extensions (Sitemap, YahooMedia,
FeedSync, iTunes, LiveJournal, SimpleList, and others)
- Update test files to use strongly-typed CompareTo method
- Remove tests for ArgumentException on wrong type (no longer
applicable with generic interface)
The CompareTo method signature changes from:
public int CompareTo(object? obj)
to:
public int CompareTo(TypeName? other)
This improves type safety by eliminating runtime type checking
and the need to throw ArgumentException for incompatible types.
…erators Introduce a marker interface and extension class to provide comparison operators (<, >, <=, >=) for 72 types implementing IComparable<T>. Changes: - Add IComparisonOperators marker interface - Add ComparisonOperatorExtensions with generic comparison operators - Implement IComparisonOperators on 72 types across Common, Core, Extensions - Remove redundant per-type comparison operators (keep == and != only) - Add 72 test classes covering all extension operator scenarios (1,440 tests) This reduces code duplication by ~2,800 lines while maintaining identical behavior through the centralized extension operators.
- Replace verbose type checks with `is` pattern in 26 MatchByType methods - Remove unnecessary .ToCharArray() from String.Split calls (9 instances) - Convert array initializations to collection expressions in SyndicationDateTimeUtility - Replace string.Format(null, ...) with string interpolation (45+ instances) - Simplify Context properties using field-backed properties (C# 13) in 8 extensions - Add generic enum reflection utilities to EnumerationMetadataAttribute - Consolidate repeated enum reflection patterns in iTunes/YahooMedia extensions Changes reduce total lines by ~300
Replace bitwise OR operator (|=) with proper conditional comparison logic in all CompareTo methods. This preserves correct ordering semantics by only comparing subsequent fields when the current comparison result is 0. Fixed patterns: - YahooMediaUtility: Fixed in CompareCommonObjectEntities, CompareCommonObjectEntityClasses, CompareCommonObjectEntityCollections, and all CompareSequence overloads - YahooMediaRestriction, YahooMediaThumbnail, YahooMediaTextConstruct, YahooMediaText, YahooMediaPlayer: Fixed consecutive property comparisons - YahooMediaContent: Fixed comparison of 15+ properties - FeedSynchronizationRelatedInformation: Fixed comparison of Link, Title, RelationType - FeedSynchronizationSyndicationExtension: Fixed comparison of extension properties and nested context - FeedSynchronizationSharingInformation: Fixed in both CompareSequence and CompareTo methods Co-Authored-By: Claude Opus 4.5 <[email protected]>
- Replace verbose type checks with `is` pattern in 26 MatchByType methods - Remove unnecessary .ToCharArray() from String.Split calls (9 instances) - Convert array initializations to collection expressions in SyndicationDateTimeUtility - Replace string.Format(null, ...) with string interpolation (45+ instances) - Simplify Context properties using field-backed properties (C# 13) in 8 extensions - Add generic enum reflection utilities to EnumerationMetadataAttribute - Consolidate repeated enum reflection patterns in iTunes/YahooMedia extensions Changes reduce total lines by ~300
- Introduce generic CompareSequence<T> in ComparisonUtility.cs, consolidating
8 type-specific overloads into a single generic method with Func<T,T,int>
- Remove duplicate <, >, <=, >= operators from 47 classes now using C# 14
extension operators via IComparisonOperators marker interface
- Apply C# 14 field keyword to 55+ properties, eliminating backing field
boilerplate in RSS, Atom, iTunes, YahooMedia, and other extension classes
- Consolidate 9 domain-specific CompareSequence methods to use the generic
ComparisonUtility.CompareSequence<T>
- Replace exception-based Version parsing with Version.TryParse in
SyndicationResourceMetadata.cs
…d syntax updates
Replace enum reflection with generic EnumerationMetadataAttribute methods
- Replace 30+ line manual reflection loops with single-line GetAlternateValue<T>()
and GetEnumByAlternateValue<T>() calls across 15 files
- Affected: AtomTextConstruct, RssCloud, BlogMLTextConstruct, YahooMedia*,
FeedSync*, DublinCore*, LiveJournalSecurity, XmlRpcClient, and more
Extract FindExtension to centralized extension method
- Create ExtensibleSyndicationObjectExtensions.cs with shared implementation
- Remove duplicate FindExtension methods from 43 implementing classes
- Update IExtensibleSyndicationObject interface (method moved to extension)
Null-conditional operator modernization
- Replace `x != null ? x.Method() : default` with `x?.Method() ?? default`
- Updated 12 files across Extensions and Core projects
Property expression body modernization
- Convert `get { return field; }` to `get => field;`
- Updated BlogML*, ApmlUtility, RsdUtility, and other files
Results: 105 files changed, 302 insertions, 2,596 deletions (-2,294 net)
…or consistency across multiple classes in the Argotic.Extensions library.
…tring'" This reverts commit 721007a which contained a faulty find-and-replace that corrupted code by replacing "o(RssEnclosur" with "cref=\"string\"" in method signatures and code blocks. Co-Authored-By: Claude Opus 4.5 <[email protected]>
…or consistency across multiple classes in the Argotic.Extensions library.
…odel repository link
…tions for various measures
Code Coverage Summary Report - Linux (No TFM)Summary
CoverageArgotic.Common - 79.1%
Argotic.Core - 65.5%
Argotic.Extensions - 72.5%
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
No description provided.