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

Revert ABI breakage in kiwix::Downloader::getDownloadIds() #999

Merged
merged 1 commit into from
Sep 14, 2023

Conversation

legoktm
Copy link
Member

@legoktm legoktm commented Sep 14, 2023

Changing it to be const is an ABI change since the symbol changes from _ZN5... to _ZNK5... (addition of "K").

The other changes made in 18b7b5f are probably fine to keep since they don't appear to be used from what I can tell.

Fixes #998.

@codecov
Copy link

codecov bot commented Sep 14, 2023

Codecov Report

❗ No coverage uploaded for pull request base (version_12.1.1@e461f2b). Click here to learn what that means.
Patch has no changes to coverable lines.

Additional details and impacted files
@@                Coverage Diff                @@
##             version_12.1.1     #999   +/-   ##
=================================================
  Coverage                  ?   38.87%           
=================================================
  Files                     ?       58           
  Lines                     ?     4000           
  Branches                  ?     2200           
=================================================
  Hits                      ?     1555           
  Misses                    ?     1101           
  Partials                  ?     1344           

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

@legoktm
Copy link
Member Author

legoktm commented Sep 14, 2023

The only thing I'm not sure about is if there's a plan to use this function in other const stuff and whether this causes issues with that.

@legoktm legoktm mentioned this pull request Sep 14, 2023
Changing it to be `const` is an ABI change since the symbol changes from
`_ZN5...` to `_ZNK5...` (addition of "K").

The other changes made in 18b7b5f are probably fine to keep since they
don't appear to be used from what I can tell.

Fixes #998.
@mgautierfr mgautierfr changed the base branch from main to version_12.1.1 September 14, 2023 11:30
@mgautierfr
Copy link
Member

LGTM, Thanks

Rebased on branch version_12.1.1

@mgautierfr mgautierfr merged commit 45b3cfc into version_12.1.1 Sep 14, 2023
12 checks passed
@mgautierfr mgautierfr deleted the const-abi branch September 14, 2023 11:42
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.

ABI break in 12.1.0
2 participants