Refactor converters, add unit tests, and enhance CI workflows - #12
Conversation
…ML formats; add no data message in PDF output
…include no data message handling
…and adjusting code style for better readability
… CSV output formatting
|
You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool. What Enabling Code Scanning Means:
For more information about GitHub Code Scanning, check out the documentation. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
CodeQL found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
There was a problem hiding this comment.
Pull request overview
This PR is a wide-ranging modernization of the repository: it restructures the project into src/tests/samples, migrates to .NET 9 with Central Package Management, adds new CI/CodeQL/NuGet-publish GitHub Actions workflows, introduces .editorconfig/.gitattributes/Directory.Build.props/Directory.Packages.props, deletes the legacy test suite and replaces it with a much broader NUnit-based test suite, refactors many converters/readers/writers for code-analysis cleanliness (static helpers, InvariantCulture, StringComparison.Ordinal, replacing generic Exception with InvalidDataException/InvalidOperationException), and rewrites the console sample as an interactive demo. FileConverter is also tweaked to hold concrete InMemoryConverter/StreamConverter instances and route all Convert* methods through them.
Changes:
- New CI/CD:
ci.yml,codeql.yml,nuget-publish.yml(+WORKFLOWS.md), removal ofdotnet.yml. - Repo structure / config:
src/tests/sampleslayout,Directory.Build.props,Directory.Packages.props,.editorconfig,.gitattributes, updated.gitignore, newnuget.config. - Library/test refactor: many converters made static-helper-clean and culture-invariant;
XmlToCsvConverternow returns empty CSV instead of throwing on empty input and prepends root-element attributes as columns;CsvToWordConverter/CsvToPdfConverteradd a "No data available" message; new comprehensive NUnit tests for every Csv*/Xml* converter.
Reviewed changes
Copilot reviewed 67 out of 87 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
.editorconfig, .gitattributes, .gitignore, nuget.config |
New/updated repo-wide formatting, line-ending, ignore, and NuGet source config. |
Directory.Build.props, Directory.Packages.props |
Centralized build settings (.NET 9, nullable, analyzers) and central package versions. |
.github/workflows/{ci,codeql,nuget-publish}.yml, .github/WORKFLOWS.md, removed dotnet.yml |
New CI, security-scan, and release workflows replacing legacy GitVersion pipeline. |
FileConversionLibrary.sln, removed old FileConversionLibrary* projects, new src/, tests/, samples/ projects |
Solution restructured into src/tests/samples layout. |
src/FileConversionLibrary/FileConversionLibrary.csproj |
New csproj using CPM; v1.8.0; suppresses specific warnings; packs README/icon. |
src/FileConversionLibrary/FileConverter.cs |
Routes file conversions through InMemoryConverter; uses concrete types; reflows long signatures. |
src/FileConversionLibrary/Converters/*.cs |
Many helpers made static; InvariantCulture/Ordinal usages; XmlToCsvConverter adds root-attribute columns and returns empty CSV on empty input; CsvToWord/CsvToPdf add no-data messages; misc cleanup. |
src/FileConversionLibrary/Readers/*, Writers/*, Models/*, Interfaces/*, Exceptions/*, Factories/ConverterFactory.cs |
BOM removal, static helpers, replace Exception with specific exception types, formatting/whitespace cleanup, XmlData/CsvData moved under src/. |
tests/FileConversionLibrary.Tests/* |
New NUnit test project using CPM; broad test coverage for all converters; old tests removed. |
samples/FileConversionLibraryConsoleExample/* |
New interactive sample replacing the old console example. |
README.md |
Reformatted (line-wrap, bullet style) and updated content. |
Comments suppressed due to low confidence (2)
src/FileConversionLibrary/Converters/XmlToCsvConverter.cs:326
- These lines use unnecessarily fully-qualified Linq/collection types (e.g.,
System.Linq.Enumerable.ToList(System.Linq.Enumerable.Select(...)),System.Collections.Generic.List<System.Xml.Linq.XAttribute>). SinceImplicitUsingsis enabled (viaDirectory.Build.props),using System.Linq;is already available and the rest of this file consistently uses extension-method syntax likeinput.Headers.Select(...)andnew List<...>(). This verbose style is inconsistent with the surrounding code and harder to read.
src/FileConversionLibrary/Converters/CsvToWordConverter.cs:373 - The "No data available" paragraph is appended to the body after the (empty) table has already been appended. When
Rows.Count == 0, the document will contain both an empty<w:tbl>(with only the header row, ifaddHeaderRowis true, or completely empty otherwise) and the "No data available" message. Consider either skipping the table append when there are no rows, or moving this check earlier so the table is not added at all in the no-data case.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Assert.That(xml, Does.Contain("1")); | ||
| } |
| var result = _converter.Convert(_simpleXmlData); | ||
| var lines = result.Split(new[] { '\r', '\n' }, StringSplitOptions.RemoveEmptyEntries); | ||
| Assert.That(lines.Length, Is.GreaterThanOrEqualTo(2)); // At least header + 1 row | ||
| Assert.That(lines[0], Does.Contain("id")); // Attribute with @ prefix |
This pull request introduces a comprehensive overhaul of the repository's build, test, and release infrastructure, focusing on modernizing the CI/CD pipeline, enforcing code style consistency, and centralizing configuration. The changes include new GitHub Actions workflows for CI, security scanning, and NuGet publishing, the addition of repository-wide configuration files for code style and package management, and the removal of legacy test files and workflows.
CI/CD Pipeline Modernization
ci.yml), CodeQL security scanning (codeql.yml), and automated NuGet publishing (nuget-publish.yml). These workflows use .NET 9, run on both pushes and pull requests, and provide build, test, code coverage, and security analysis automation. The NuGet workflow supports both release-triggered and manual publishing with version extraction and artifact upload. [1] [2] [3].NETworkflow (dotnet.yml) that used GitVersion and .NET 8, consolidating all CI/CD processes into the new, streamlined workflows..github/WORKFLOWS.md, including setup instructions, requirements, and status badge integration for the README.Repository Configuration and Code Style
.editorconfigfile to enforce consistent code formatting, indentation, C# code style, and naming conventions across the repository..gitattributesfile to ensure consistent line endings (CRLF for most files, LF for shell scripts), text/binary file handling, and source file diff configuration.Build and Dependency Management
Directory.Build.propsto centralize .NET build configuration, targeting .NET 9, enabling nullable reference types, strict warnings, and code analysis settings, with relaxed rules for test and sample projects.Directory.Packages.propsto enable central package version management and specify versions for all core library and test dependencies.Test Suite Cleanup
CsvToJsonConverterTests.csandCsvToPdfConverterTests.csfrom the test project, likely in preparation for a new or refactored test suite. [1] [2]