Upgrade to .NET 10#144
Open
HowardvanRooijen wants to merge 91 commits into
Open
Conversation
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.
Get-EndjinGist zf prautoflow Get-EndjinGist zf dotnet
Test Results3 703 tests +3 589 3 703 ✅ +3 609 7s ⏱️ +7s Results for commit 1df1a8a. ± Comparison against base commit 37fc3cf. This pull request removes 114 and adds 3703 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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
- 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 <noreply@anthropic.com>
…or consistency across multiple classes in the Argotic.Extensions library.
…odel repository link
…tions for various measures
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 81 out of 513 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 81 out of 514 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 81 out of 514 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+143
to
+162
| public static string GetAlternateValue<TEnum>(TEnum value) where TEnum : struct, Enum | ||
| { | ||
| return EnumMetadataCache<TEnum>.EnumToAlternateValue.GetValueOrDefault(value, string.Empty); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Returns a hash code for the current instance. | ||
| /// </summary> | ||
| /// <returns>A 32-bit signed integer hash code.</returns> | ||
| public override int GetHashCode() | ||
| { | ||
| char[] charArray = this.ToString().ToCharArray(); | ||
| /// <summary> | ||
| /// Gets the enum value that corresponds to the specified alternate value name. | ||
| /// </summary> | ||
| /// <typeparam name="TEnum">The enum type.</typeparam> | ||
| /// <param name="name">The alternate value name to search for.</param> | ||
| /// <param name="defaultValue">The default value to return if not found.</param> | ||
| /// <returns>The enum value if found, otherwise the default value.</returns> | ||
| /// <exception cref="ArgumentNullException">The <paramref name="name"/> is a null reference.</exception> | ||
| /// <exception cref="ArgumentException">The <paramref name="name"/> is an empty string.</exception> | ||
| public static TEnum GetEnumByAlternateValue<TEnum>(string name, TEnum defaultValue) where TEnum : struct, Enum | ||
| { | ||
| ArgumentException.ThrowIfNullOrEmpty(name); | ||
|
|
||
| return charArray.GetHashCode(); | ||
| } | ||
| return EnumMetadataCache<TEnum>.AlternateValueToEnum.GetValueOrDefault(name, defaultValue); | ||
| } |
There was a problem hiding this comment.
GetAlternateValue<TEnum> returns string.Empty when no mapping exists, but GetEnumByAlternateValue<TEnum> throws on empty strings. This makes round-tripping brittle and prevents representing enums whose AlternateValue is intentionally empty (common for "None"/default cases). Consider returning defaultValue when name is null/empty (instead of throwing), and (if needed) include empty-string alternates in the reverse mapping.
Comment on lines
+208
to
+212
| public static bool operator ==(XmlRpcArrayValue first, XmlRpcArrayValue second) | ||
| { | ||
| if (first is null) return second is null; | ||
| return first.Equals(second); | ||
| } |
There was a problem hiding this comment.
operator == accepts non-nullable parameters but explicitly handles null. With nullable reference types enabled, it’s better for both API clarity and caller warnings to declare the operands as nullable (XmlRpcArrayValue? first, XmlRpcArrayValue? second) and keep the same null-handling logic.
Comment on lines
+60
to
+75
| int counter = 0; | ||
| while (urlIterator.MoveNext()) | ||
| { | ||
| SitemapUrl url = new(); | ||
| counter++; | ||
|
|
||
| if (url.Load(urlIterator.Current, this.Settings)) | ||
| { | ||
| if (this.Settings.RetrievalLimit != 0 && counter > this.Settings.RetrievalLimit) | ||
| { | ||
| break; | ||
| } | ||
|
|
||
| resource.Urls.Add(url); | ||
| } | ||
| } |
There was a problem hiding this comment.
The retrieval limit check happens after url.Load(...), so the adapter may still parse one extra entry beyond the requested limit (and could do so even if it won’t be added). To align with the intent of RetrievalLimit and avoid extra work, move the limit check before calling Load.
Comment on lines
+102
to
+117
| int counter = 0; | ||
| while (sitemapIterator.MoveNext()) | ||
| { | ||
| SitemapIndexEntry entry = new(); | ||
| counter++; | ||
|
|
||
| if (entry.Load(sitemapIterator.Current)) | ||
| { | ||
| if (this.Settings.RetrievalLimit != 0 && counter > this.Settings.RetrievalLimit) | ||
| { | ||
| break; | ||
| } | ||
|
|
||
| resource.Sitemaps.Add(entry); | ||
| } | ||
| } |
There was a problem hiding this comment.
Same pattern as the sitemap URL loop: the retrieval limit is enforced only after entry.Load(...), which can still process one extra entry beyond the limit. Consider checking RetrievalLimit before Load to avoid unnecessary parsing.
Comment on lines
+181
to
+191
| public int CompareTo(XmlRpcStructureMember? other) | ||
| { | ||
| if (other is null) | ||
| { | ||
| char[] charArray = this.ToString().ToCharArray(); | ||
|
|
||
| return charArray.GetHashCode(); | ||
| return 1; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Determines if operands are equal. | ||
| /// </summary> | ||
| /// <param name="first">Operand to be compared.</param> | ||
| /// <param name="second">Operand to compare to.</param> | ||
| /// <returns><b>true</b> if the values of its operands are equal, otherwise; <b>false</b>.</returns> | ||
| public static bool operator ==(XmlRpcStructureMember first, XmlRpcStructureMember second) | ||
| { | ||
| if (object.Equals(first, null) && object.Equals(second, null)) | ||
| { | ||
| return true; | ||
| } | ||
| else if (object.Equals(first, null) && !object.Equals(second, null)) | ||
| { | ||
| return false; | ||
| } | ||
| int result = string.Compare(this.ToString(), other.ToString(), StringComparison.Ordinal); | ||
|
|
||
| return first.Equals(second); | ||
| } | ||
| return result; | ||
| } |
There was a problem hiding this comment.
CompareTo serializes both instances to XML (ToString()) on every comparison, which is expensive and creates significant allocations when sorting or using relational operators. Prefer comparing the actual fields (e.g., Name first, then Value) without generating XML.
| /// Loads this <see cref="XmlRpcStructureMember"/> using the supplied <see cref="XPathNavigator"/>. | ||
| /// </summary> | ||
| /// <param name="source">The <see cref="XPathNavigator"/> to extract information from.</param> | ||
| /// <returns><b>true</b> if the <see cref="XmlRpcStructureMember"/> was initialized using the supplied <paramref name="source"/>, Otherwise, <b>false</b>.</returns> |
There was a problem hiding this comment.
In XML docs, mid-sentence capitalization is inconsistent here. Consider changing 'Otherwise' to 'otherwise'.
Suggested change
| /// <returns><b>true</b> if the <see cref="XmlRpcStructureMember"/> was initialized using the supplied <paramref name="source"/>, Otherwise, <b>false</b>.</returns> | |
| /// <returns><b>true</b> if the <see cref="XmlRpcStructureMember"/> was initialized using the supplied <paramref name="source"/>, otherwise, <b>false</b>.</returns> |
…nit tests for null/empty cases to return None
Code Coverage Summary Report - Linux (No TFM)Summary
CoverageArgotic.Common - 78.6%
Argotic.Core - 66.5%
Argotic.Extensions - 72.5%
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 81 out of 515 changed files in this pull request and generated 9 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
1
to
7
| <Project Sdk="Microsoft.NET.Sdk"> | ||
|
|
||
| <PropertyGroup> | ||
| <TargetFrameworks>netstandard2.0;netstandard2.1;net8.0;net9.0;</TargetFrameworks> | ||
| <Product>Argotic Syndication Framework</Product> | ||
| <Copyright>Copyright © Brian William Kuhn 2008</Copyright> | ||
| <Authors>Brian William Kuhn, endjin</Authors> | ||
| </PropertyGroup> | ||
|
|
||
| <PropertyGroup> | ||
| <PackageLicenseExpression>Apache-2.0</PackageLicenseExpression> | ||
| <PackageDescription></PackageDescription> | ||
| <PackageTags></PackageTags> | ||
| <PackageReleaseNotes></PackageReleaseNotes> | ||
| <IncludeSymbols>true</IncludeSymbols> | ||
| <SymbolPackageFormat>snupkg</SymbolPackageFormat> | ||
| <PackageDescription>Main syndication library for RSS, Atom, OPML, APML, BlogML, RSD, and Sitemap formats. Provides reading, writing, and manipulation of web content syndication feeds.</PackageDescription> | ||
| <GenerateDocumentationFile>true</GenerateDocumentationFile> | ||
| </PropertyGroup> |
There was a problem hiding this comment.
The project no longer declares a target framework (<TargetFramework> or <TargetFrameworks>). SDK-style projects require this, so this change will break builds. Add a .NET 10 target (e.g., net10.0) or restore multi-targeting as intended for this PR.
Comment on lines
1
to
7
| <Project Sdk="Microsoft.NET.Sdk"> | ||
|
|
||
| <PropertyGroup> | ||
| <TargetFrameworks>netstandard2.0;netstandard2.1;net8.0;net9.0;</TargetFrameworks> | ||
| <Product>Argotic Syndication Framework</Product> | ||
| <Copyright>Copyright © Brian William Kuhn 2008</Copyright> | ||
| <Authors>Brian William Kuhn, endjin</Authors> | ||
| <PackageDescription>Core interfaces and utilities for the Argotic Syndication Framework, including SyndicationEncodingUtility, SyndicationDateTimeUtility, and SyndicationDiscoveryUtility.</PackageDescription> | ||
| <GenerateDocumentationFile>true</GenerateDocumentationFile> | ||
| </PropertyGroup> |
There was a problem hiding this comment.
This project also no longer specifies <TargetFramework>/<TargetFrameworks>, which will prevent it from compiling/packing. Add the intended .NET 10 target (or restore the previous multi-targeting) to make the project file valid.
Comment on lines
+5
to
+62
| /// Provides extension comparison operators for types implementing <see cref="IComparable{T}"/>. | ||
| /// </summary> | ||
| /// <remarks> | ||
| /// C# 14 extension operators for relational comparisons (<, >, <=, >=). | ||
| /// Note: == and != cannot be extension operators for reference types due to | ||
| /// predefined reference equality taking precedence. | ||
| /// </remarks> | ||
| public static class ComparisonOperatorExtensions | ||
| { | ||
| extension<T>(T) where T : class, IComparable<T>, IComparisonOperators | ||
| { | ||
| /// <summary> | ||
| /// Determines if first operand is less than second operand. | ||
| /// </summary> | ||
| /// <param name="first">Operand to be compared.</param> | ||
| /// <param name="second">Operand to compare to.</param> | ||
| /// <returns><b>true</b> if the first operand is less than the second, otherwise; <b>false</b>.</returns> | ||
| public static bool operator <(T first, T second) | ||
| { | ||
| if (first is null) return second is not null; | ||
| return first.CompareTo(second) < 0; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Determines if first operand is greater than second operand. | ||
| /// </summary> | ||
| /// <param name="first">Operand to be compared.</param> | ||
| /// <param name="second">Operand to compare to.</param> | ||
| /// <returns><b>true</b> if the first operand is greater than the second, otherwise; <b>false</b>.</returns> | ||
| public static bool operator >(T first, T second) | ||
| { | ||
| if (first is null) return false; | ||
| return first.CompareTo(second) > 0; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Determines if first operand is less than or equal to the second operand. | ||
| /// </summary> | ||
| /// <param name="first">Operand to be compared.</param> | ||
| /// <param name="second">Operand to compare to.</param> | ||
| /// <returns><b>true</b> if the first operand is less than or equal to the second, otherwise; <b>false</b>.</returns> | ||
| public static bool operator <=(T first, T second) | ||
| { | ||
| if (first is null) return true; | ||
| return first.CompareTo(second) <= 0; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Determines if first operand is greater than or equal to the second operand. | ||
| /// </summary> | ||
| /// <param name="first">Operand to be compared.</param> | ||
| /// <param name="second">Operand to compare to.</param> | ||
| /// <returns><b>true</b> if the first operand is greater than or equal to the second, otherwise; <b>false</b>.</returns> | ||
| public static bool operator >=(T first, T second) | ||
| { | ||
| if (first is null) return second is null; | ||
| return first.CompareTo(second) >= 0; | ||
| } |
There was a problem hiding this comment.
This uses the extension<T>(T) syntax (extension operators / extension type blocks), which requires a very new C# language version and typically a preview compiler setting. If the repo/build isn't explicitly enabling the required LangVersion/preview features, this will fail to compile. Consider either (1) setting the project(s) LangVersion appropriately, or (2) replacing this with conventional static extension methods (or regular operator overloads on the types) to avoid depending on preview syntax.
Suggested change
| /// Provides extension comparison operators for types implementing <see cref="IComparable{T}"/>. | |
| /// </summary> | |
| /// <remarks> | |
| /// C# 14 extension operators for relational comparisons (<, >, <=, >=). | |
| /// Note: == and != cannot be extension operators for reference types due to | |
| /// predefined reference equality taking precedence. | |
| /// </remarks> | |
| public static class ComparisonOperatorExtensions | |
| { | |
| extension<T>(T) where T : class, IComparable<T>, IComparisonOperators | |
| { | |
| /// <summary> | |
| /// Determines if first operand is less than second operand. | |
| /// </summary> | |
| /// <param name="first">Operand to be compared.</param> | |
| /// <param name="second">Operand to compare to.</param> | |
| /// <returns><b>true</b> if the first operand is less than the second, otherwise; <b>false</b>.</returns> | |
| public static bool operator <(T first, T second) | |
| { | |
| if (first is null) return second is not null; | |
| return first.CompareTo(second) < 0; | |
| } | |
| /// <summary> | |
| /// Determines if first operand is greater than second operand. | |
| /// </summary> | |
| /// <param name="first">Operand to be compared.</param> | |
| /// <param name="second">Operand to compare to.</param> | |
| /// <returns><b>true</b> if the first operand is greater than the second, otherwise; <b>false</b>.</returns> | |
| public static bool operator >(T first, T second) | |
| { | |
| if (first is null) return false; | |
| return first.CompareTo(second) > 0; | |
| } | |
| /// <summary> | |
| /// Determines if first operand is less than or equal to the second operand. | |
| /// </summary> | |
| /// <param name="first">Operand to be compared.</param> | |
| /// <param name="second">Operand to compare to.</param> | |
| /// <returns><b>true</b> if the first operand is less than or equal to the second, otherwise; <b>false</b>.</returns> | |
| public static bool operator <=(T first, T second) | |
| { | |
| if (first is null) return true; | |
| return first.CompareTo(second) <= 0; | |
| } | |
| /// <summary> | |
| /// Determines if first operand is greater than or equal to the second operand. | |
| /// </summary> | |
| /// <param name="first">Operand to be compared.</param> | |
| /// <param name="second">Operand to compare to.</param> | |
| /// <returns><b>true</b> if the first operand is greater than or equal to the second, otherwise; <b>false</b>.</returns> | |
| public static bool operator >=(T first, T second) | |
| { | |
| if (first is null) return second is null; | |
| return first.CompareTo(second) >= 0; | |
| } | |
| /// Provides comparison extension methods for types implementing <see cref="IComparable{T}"/>. | |
| /// </summary> | |
| public static class ComparisonOperatorExtensions | |
| { | |
| /// <summary> | |
| /// Determines if first operand is less than second operand. | |
| /// </summary> | |
| /// <param name="first">Operand to be compared.</param> | |
| /// <param name="second">Operand to compare to.</param> | |
| /// <returns><b>true</b> if the first operand is less than the second, otherwise; <b>false</b>.</returns> | |
| public static bool IsLessThan<T>(this T first, T second) where T : class, IComparable<T>, IComparisonOperators | |
| { | |
| if (first is null) return second is not null; | |
| return first.CompareTo(second) < 0; | |
| } | |
| /// <summary> | |
| /// Determines if first operand is greater than second operand. | |
| /// </summary> | |
| /// <param name="first">Operand to be compared.</param> | |
| /// <param name="second">Operand to compare to.</param> | |
| /// <returns><b>true</b> if the first operand is greater than the second, otherwise; <b>false</b>.</returns> | |
| public static bool IsGreaterThan<T>(this T first, T second) where T : class, IComparable<T>, IComparisonOperators | |
| { | |
| if (first is null) return false; | |
| return first.CompareTo(second) > 0; | |
| } | |
| /// <summary> | |
| /// Determines if first operand is less than or equal to the second operand. | |
| /// </summary> | |
| /// <param name="first">Operand to be compared.</param> | |
| /// <param name="second">Operand to compare to.</param> | |
| /// <returns><b>true</b> if the first operand is less than or equal to the second, otherwise; <b>false</b>.</returns> | |
| public static bool IsLessThanOrEqualTo<T>(this T first, T second) where T : class, IComparable<T>, IComparisonOperators | |
| { | |
| if (first is null) return true; | |
| return first.CompareTo(second) <= 0; | |
| } | |
| /// <summary> | |
| /// Determines if first operand is greater than or equal to the second operand. | |
| /// </summary> | |
| /// <param name="first">Operand to be compared.</param> | |
| /// <param name="second">Operand to compare to.</param> | |
| /// <returns><b>true</b> if the first operand is greater than or equal to the second, otherwise; <b>false</b>.</returns> | |
| public static bool IsGreaterThanOrEqualTo<T>(this T first, T second) where T : class, IComparable<T>, IComparisonOperators | |
| { | |
| if (first is null) return second is null; | |
| return first.CompareTo(second) >= 0; |
| # C# files | ||
| [*.cs] | ||
|
|
||
| # C# 12 Language features |
There was a problem hiding this comment.
The repo configuration claims 'C# 12 Language features', but the PR introduces syntax that appears to require a newer C# version (e.g., extension operators). To avoid confusion and build breaks across environments/CI, update the repo configuration and/or project files to explicitly align the intended C# language version (and whether preview is required).
Suggested change
| # C# 12 Language features | |
| # C# language style preferences | |
| # Note: compiler language version should be enforced in project/SDK settings, not in .editorconfig. |
| { | ||
| return false; | ||
| } | ||
| int result = string.Compare(this.ToString(), other.ToString(), StringComparison.Ordinal); |
There was a problem hiding this comment.
Both CompareTo and GetHashCode call ToString(), which serializes the object to XML (allocations + IO over MemoryStream/XmlWriter). This makes comparisons/hashing unexpectedly expensive and can become a bottleneck when these objects are used in sorting or hash-based collections. Prefer comparing/hashing stable fields directly (e.g., Name plus a value-specific comparer/hash) or cache the serialized representation if XML-string identity is the intended semantics.
| /// <summary> | ||
| /// Gets or sets a value indicating if syndication resource persist operations should attempt to minimize the physical size of the resulting output. | ||
| /// </summary> | ||
| /// <value><b>true</b> if output size should be as small as possible; Otherwise, <b>false</b>. The default value is <b>false</b>.</value> |
There was a problem hiding this comment.
In XML doc comments, 'Otherwise' mid-sentence should be lowercase ('otherwise') for correct grammar.
| /// Gets or sets a value indicating if auto-detection of supported syndication extensions is enabled. | ||
| /// </summary> | ||
| /// <value> | ||
| /// <b>true</b> if the syndication extensions supported by the load operation are automatically determined based on the XML namespaces declared on a syndication resource; Otherwise, <b>false</b>. |
There was a problem hiding this comment.
In XML doc comments, 'Otherwise' mid-sentence should be lowercase ('otherwise') for correct grammar.
Suggested change
| /// <b>true</b> if the syndication extensions supported by the load operation are automatically determined based on the XML namespaces declared on a syndication resource; Otherwise, <b>false</b>. | |
| /// <b>true</b> if the syndication extensions supported by the load operation are automatically determined based on the XML namespaces declared on a syndication resource; otherwise, <b>false</b>. |
| charset = utf-8 | ||
| indent_style = space | ||
| indent_size = 4 | ||
| end_of_line = lf |
There was a problem hiding this comment.
.editorconfig enforces LF for all files, but .gitattributes enforces CRLF for *.ps1/*.cmd/*.bat (and others). This conflict can cause persistent line-ending churn depending on tooling and checkout settings. Align the two sources of truth (either rely on .gitattributes for EOL normalization or mirror the same per-extension EOL rules in .editorconfig).
Comment on lines
+18
to
+34
| *.sh text eol=lf | ||
| *.ps1 text eol=crlf | ||
| *.cmd text eol=crlf | ||
| *.bat text eol=crlf | ||
|
|
||
| # Declare files that will always have LF line endings on checkout. | ||
| .gitattributes text eol=lf | ||
| .gitignore text eol=lf | ||
| *.sh text eol=lf | ||
|
|
||
| # Declare files that will always have CRLF line endings on checkout. | ||
| *.sln text eol=crlf | ||
| *.slnx text eol=crlf | ||
| *.cmd text eol=crlf | ||
| *.bat text eol=crlf | ||
| *.ps1 text eol=crlf | ||
|
|
There was a problem hiding this comment.
These per-extension EOL rules conflict with the global LF rule in .editorconfig. To avoid repeated diffs and inconsistent formatting across contributors/CI, consider standardizing EOL policy in one place (or ensuring .editorconfig mirrors these per-extension settings).
Suggested change
| *.sh text eol=lf | |
| *.ps1 text eol=crlf | |
| *.cmd text eol=crlf | |
| *.bat text eol=crlf | |
| # Declare files that will always have LF line endings on checkout. | |
| .gitattributes text eol=lf | |
| .gitignore text eol=lf | |
| *.sh text eol=lf | |
| # Declare files that will always have CRLF line endings on checkout. | |
| *.sln text eol=crlf | |
| *.slnx text eol=crlf | |
| *.cmd text eol=crlf | |
| *.bat text eol=crlf | |
| *.ps1 text eol=crlf | |
| # Declare files that will always have LF line endings on checkout. | |
| .gitattributes text eol=lf | |
| .gitignore text eol=lf | |
| *.sh text eol=lf | |
| *.ps1 text eol=lf | |
| *.cmd text eol=lf | |
| *.bat text eol=lf | |
| *.sln text eol=lf | |
| *.slnx text eol=lf |
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
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.