Skip to content
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

Update project for initial release #1

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

paulirwin
Copy link

This PR updates the project in preparation for our initial release under the Apache GitHub organization. It includes:

  • Updates NuGet packages and target framework for unit test project
  • Adds Sample project to test and debug analyzers
  • Adds a launchSettings.json file to allow for debugging the analyzers, and removes VSIX support
  • Fixes a failing unit test
  • Fixes Roslyn meta-analyzer warnings (as errors)
  • Adds GitHub Actions file
  • Removes unused CodeFixes project (these will go into the same analyzers project)

Copy link
Contributor

@NightOwl888 NightOwl888 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR.

I confirmed I was able to run the tests and hit a breakpoint after the changes.

I noticed that Microsoft uses .ruleset files to group rules together. Since we will have specific rules that will need to be enabled when we upgrade to the next version of Lucene (LuceneDev1000-LuceneDev1004 are all good examples of these) it would be good to be able to group them.

  1. Default - for general development
  2. Upgrade - only useful when upgrading Lucene
  3. Code Search - only useful when searching for complex code patterns that are specific to Lucene's design that are too difficult to do using Find and Replace

I suspect there will be other categories as the project develops.

According to ChatGPT we can use .ruleset files for this purpose: https://chatgpt.com/share/67cfaeeb-72c4-8005-a530-afb1e27cec94

image

There is also some other stuff to address. Please see the inline comments.

@NightOwl888
Copy link
Contributor

One thing I forgot about in the review was that since this project will be a private dependency of all of the Lucene.Net projects, it should be strong named to avoid the build warning. Ideally, we would use the same SNK from the lucenenet repo, but I see no issue with copying it here, since it is unlikely we will ever change it.

@paulirwin
Copy link
Author

@NightOwl888 I split out creating rulesets as apache/lucenenet#1143

Ready for re-review.

@paulirwin paulirwin requested a review from NightOwl888 March 16, 2025 23:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants