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

add __declspec(dllexport) to class #783

Merged
merged 1 commit into from Apr 25, 2023
Merged

Conversation

xiaoyifang
Copy link
Contributor

@xiaoyifang xiaoyifang commented Apr 13, 2023

fixes #780
related to #741

@codecov
Copy link

codecov bot commented Apr 13, 2023

Codecov Report

Patch coverage: 87.50% and no project coverage change.

Comparison is base (6210433) 56.07% compared to head (1b9f748) 56.07%.

❗ Current head 1b9f748 differs from pull request most recent head 841ffdf. Consider uploading reports for the commit 841ffdf to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #783   +/-   ##
=======================================
  Coverage   56.07%   56.07%           
=======================================
  Files          98       98           
  Lines        4394     4394           
  Branches     1910     1910           
=======================================
  Hits         2464     2464           
  Misses        672      672           
  Partials     1258     1258           
Impacted Files Coverage Δ
include/zim/archive.h 94.82% <ø> (ø)
include/zim/blob.h 72.72% <ø> (ø)
include/zim/entry.h 100.00% <ø> (ø)
include/zim/error.h 71.42% <ø> (ø)
include/zim/writer/item.h 33.33% <ø> (ø)
include/zim/search.h 50.00% <50.00%> (ø)
include/zim/search_iterator.h 100.00% <100.00%> (ø)
include/zim/suggestion.h 100.00% <100.00%> (ø)
include/zim/suggestion_iterator.h 87.50% <100.00%> (ø)
include/zim/writer/contentProvider.h 53.84% <100.00%> (ø)
... and 1 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@xiaoyifang xiaoyifang marked this pull request as draft April 14, 2023 01:50
@xiaoyifang xiaoyifang marked this pull request as ready for review April 14, 2023 03:07
@xiaoyifang
Copy link
Contributor Author

@mgautierfr can you approve this to run the checks

@kelson42 kelson42 self-requested a review April 16, 2023 07:29
@kelson42
Copy link
Contributor

kelson42 commented Apr 16, 2023

Small comment about the naming: I would prefer DECLSPEC in place of ZIM_LIB, only my opinion.

@xiaoyifang
Copy link
Contributor Author

done

@xiaoyifang xiaoyifang force-pushed the fix/win-build branch 2 times, most recently from a44d856 to 4fc32bc Compare April 16, 2023 10:28
include/zim/error.h Outdated Show resolved Hide resolved
include/zim/blob.h Outdated Show resolved Hide resolved
@xiaoyifang xiaoyifang force-pushed the fix/win-build branch 2 times, most recently from bafd7ba to 9775e58 Compare April 17, 2023 23:39
include/zim/tools.h Show resolved Hide resolved
include/zim/uuid.h Outdated Show resolved Hide resolved
@kelson42
Copy link
Contributor

@veloman-yunkan Are we good to merge?

Copy link
Collaborator

@veloman-yunkan veloman-yunkan left a comment

Choose a reason for hiding this comment

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

Last small change and we can merge

include/zim/uuid.h Outdated Show resolved Hide resolved
include/zim/zim.h Outdated Show resolved Hide resolved
fix build dynamic library issue on Windows
@kelson42 kelson42 merged commit 34b084f into openzim:main Apr 25, 2023
31 checks passed
@xiaoyifang xiaoyifang mentioned this pull request Apr 29, 2023
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.

Correctly export public api
4 participants