Skip to content

Fixing finding licenses#15

Merged
kamaal111 merged 1 commit into
mainfrom
14-make-acknowledgements-script-removes-licenses
May 18, 2025
Merged

Fixing finding licenses#15
kamaal111 merged 1 commit into
mainfrom
14-make-acknowledgements-script-removes-licenses

Conversation

@kamaal111

@kamaal111 kamaal111 commented May 18, 2025

Copy link
Copy Markdown
Member

Pull Request Overview

This PR replaces the --scheme flag with --project, introduces functions to locate Xcode’s DerivedData and SourcePackages paths, and refactors license discovery to use pathlib.

  • Change CLI flag from --scheme to --project and update argument parsing.
  • Add get_xcode_derived_data_base and find_derived_data_for_project to resolve DerivedData.
  • Rewrite get_packages_licenses to use Path.iterdir and read license files.

@kamaal111 kamaal111 linked an issue May 18, 2025 that may be closed by this pull request
@kamaal111 kamaal111 requested a review from Copilot May 18, 2025 14:54

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull Request Overview

This PR replaces the --scheme flag with --project, introduces functions to locate Xcode’s DerivedData and SourcePackages paths, and refactors license discovery to use pathlib.

  • Change CLI flag from --scheme to --project and update argument parsing.
  • Add get_xcode_derived_data_base and find_derived_data_for_project to resolve DerivedData.
  • Rewrite get_packages_licenses to use Path.iterdir and read license files.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
src/xctools_kamaalio/actions/acknowledgments.py Updated argument parsing, added DerivedData resolution, refactored license discovery, and minor formatting updates
pyproject.toml Bumped version from 0.4.0 to 0.5.0
Comments suppressed due to low confidence (1)

src/xctools_kamaalio/actions/acknowledgments.py:218

  • [nitpick] The function name could be shortened or clarified, e.g., get_derived_data_base, since the Xcode context is already in the module name.
def get_xcode_derived_data_base() -> Path:

Comment thread src/xctools_kamaalio/actions/acknowledgments.py
Comment on lines +157 to 178
for content_path in packages_directory.iterdir():
if not content_path.is_dir():
continue

package_name = root.split("/")[-1]
for package_path in content_path.iterdir():
if not package_path.is_file() or "license" not in package_path.name.lower():
continue

license_path = os.path.join(root, "LICENSE")
with open(license_path, "r") as file:
licenses[package_name] = file.read()
licenses[content_path.name] = package_path.read_text()

Copilot AI May 18, 2025

Copy link

Choose a reason for hiding this comment

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

Current logic only searches one level deep for license files; nested LICENSE files or unconventional names may be missed. Consider using Path.rglob with a case-insensitive pattern to ensure all licenses are found.

Copilot uses AI. Check for mistakes.
def find_derived_data_for_project(project_name: str) -> Path:
base = get_xcode_derived_data_base()
pattern = str(base / f"{project_name}-*")
matches = glob.glob(pattern)

Copilot AI May 18, 2025

Copy link

Choose a reason for hiding this comment

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

Instead of glob.glob followed by sorting, consider using base.glob(f"{project_name}-*") and max() with a key on Path.stat().st_mtime to avoid the extra dependency and simplify the code.

Copilot uses AI. Check for mistakes.
@kamaal111 kamaal111 force-pushed the 14-make-acknowledgements-script-removes-licenses branch from ee736d0 to 4c507a9 Compare May 18, 2025 15:00
@kamaal111 kamaal111 marked this pull request as ready for review May 18, 2025 15:05
@kamaal111 kamaal111 merged commit 7117366 into main May 18, 2025
6 checks passed
@kamaal111 kamaal111 deleted the 14-make-acknowledgements-script-removes-licenses branch May 18, 2025 15:06
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.

Make acknowledgements script removes licenses

2 participants