Ignore packages with commit instead of version

[hbmartin]

No description provided.

[hbmartin]

@CodiumAI-Agent /review

[CodiumAI-Agent]

PR Review

⏱️ Estimated effort to review [1-5]	3, because the PR involves changes across multiple files with modifications in both logic and structure. The changes include refactoring, error handling improvements, and new method extractions which require careful review to ensure functionality remains consistent and error handling is robust.
🧪 Relevant tests	Yes
🔍 Possible issues	Error Handling: The new error handling in warn_for_new_versions_range and warn_for_new_versions captures exceptions but only logs them to stderr. This might not be sufficient for all deployment environments where more sophisticated error reporting or handling might be necessary.
🔒 Security concerns	No

Code feedback:

---

relevant file	lib/spm_version_updates/git.rb
suggestion	Consider using sort_by instead of sort! for better performance when sorting complex objects or when the sorting criteria might become more complex in the future. [medium]
relevant line	versions.sort!.reverse!

---

relevant file	lib/spm_version_updates/plugin.rb
suggestion	Extract the repeated error handling logic into a separate method to reduce duplication and improve maintainability. [important]
relevant line	rescue ArgumentError => e

---

relevant file	lib/spm_version_updates/plugin.rb
suggestion	Consider implementing a retry mechanism or a more sophisticated error recovery strategy in the new warn_for_branch method to handle transient issues when fetching the last commit. [medium]
relevant line	last_commit = Git.branch_last_commit(repository_url, branch)

---

relevant file	lib/spm_version_updates/plugin.rb
suggestion	Use a more descriptive variable name than e in the rescue blocks to improve code readability and maintainability. [medium]
relevant line	rescue ArgumentError => e

---

---

✨ Review tool usage guide:

---

Overview:
The review tool scans the PR code changes, and generates a PR review which includes several types of feedbacks, such as possible PR issues, security threats and relevant test in the PR. More feedbacks can be added by configuring the tool.

The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on any PR.

• When commenting, to edit configurations related to the review tool (pr_reviewer section), use the following template:

/review --pr_reviewer.some_config1=... --pr_reviewer.some_config2=...

• With a configuration file, use the following template:

[pr_reviewer]
some_config1=...
some_config2=...

See the review usage page for a comprehensive guide on using this tool.

[sourcery-ai]

Hey @hbmartin - I've reviewed your changes and they look great!

Here's what I looked at during the review

• 🟡 General issues: 1 issue found
• 🟢 Security: all looks good
• 🟡 Testing: 2 issues found
• 🟢 Complexity: all looks good
• 🟢 Docstrings: all looks good

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.}

[sourcery-ai]

issue (testing): Test case 'Up to next major but null version' lacks assertions for specific warnings.

Please add assertions to verify that the correct warnings are generated when the version is null. This will ensure that the new logic in warn_for_new_versions_range and warn_for_new_versions is correctly handling null or invalid versions.

[sourcery-ai]

suggestion (testing): Consider adding more test cases for error handling in version parsing.

Given the changes in error handling for version parsing in warn_for_new_versions_range and warn_for_new_versions, it would be beneficial to include tests that trigger and verify the handling of ArgumentError.

Suggested change

	it "Up to next major but null version" do
	it "Up to next major but null version" do
	expect { @my_plugin.check_for_updates("#{File.dirname(__FILE__)}/support/fixtures/PackageV1Commit.xcodeproj") }.to_not raise_error
	expect(@dangerfile.status_report[:warnings]).to eq([])
	end
	it "Does not crash or warn when resolved version is missing from xcodeproj" do
	expect { @my_plugin.check_for_updates("#{File.dirname(__FILE__)}/support/fixtures/NoResolvedVersion.xcodeproj") }.to_not raise_error
	end

[codecov]

Codecov Report

Attention: Patch coverage is 81.81818% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 93.83%. Comparing base (a36af65) to head (e860ceb).

Files	Patch %	Lines
lib/spm_version_updates/plugin.rb	80.00%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main       #8      +/-   ##
==========================================
- Coverage   95.07%   93.83%   -1.24%     
==========================================
  Files           5        5              
  Lines         142      146       +4     
==========================================
+ Hits          135      137       +2     
- Misses          7        9       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.