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

Use vendored taglib for swig #137

Merged
merged 2 commits into from
Apr 8, 2024
Merged

Use vendored taglib for swig #137

merged 2 commits into from
Apr 8, 2024

Conversation

jacobvosmaer
Copy link
Collaborator

@jacobvosmaer jacobvosmaer commented Apr 6, 2024

This is working towards deterministic rake swig (see #136).

  • let rake swig use the taglib installed by rake vendor
  • use TagLib 1.11.1 in rake vendor (the checked-in wrapper files were already based on 1.11.1)
  • normalize @SWIG comments so they don't depend on where the developer installed swig

tasks/taglib.rb Outdated Show resolved Hide resolved
tasks/taglib.rb Outdated Show resolved Hide resolved
tasks/ext.rake Outdated Show resolved Hide resolved
Copy link
Owner

@robinst robinst left a comment

Choose a reason for hiding this comment

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

LGTM, just some minor comments to be addressed.

tasks/taglib.rb Outdated Show resolved Hide resolved
tasks/ext.rake Outdated Show resolved Hide resolved
tasks/swig.rake Outdated Show resolved Hide resolved
tasks/swig.rake Show resolved Hide resolved
tasks/swig.rake Outdated Show resolved Hide resolved
This is working towards deterministic `rake swig` (see
#136).
@jacobvosmaer jacobvosmaer force-pushed the jv-swig-taglib-vendor branch 2 times, most recently from dc190f3 to f950e87 Compare April 7, 2024 11:06
.github/workflows/ci.yml Outdated Show resolved Hide resolved
@jacobvosmaer jacobvosmaer force-pushed the jv-swig-taglib-vendor branch from f950e87 to 0e6ff61 Compare April 7, 2024 11:23
@jacobvosmaer
Copy link
Collaborator Author

@robinst is there anything else we need to do in this PR before we merge?

@robinst
Copy link
Owner

robinst commented Apr 8, 2024

No, you can go ahead and merge :)

@jacobvosmaer jacobvosmaer merged commit cc749ba into main Apr 8, 2024
4 checks passed
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