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.

Sign up for GitHub

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

Jump to bottom

Replace boost filesystem with std filesystem #275

Merged
merged 1 commit into from Oct 6, 2023
Merged

Replace boost filesystem with std filesystem #275

merged 1 commit into from Oct 6, 2023

Conversation

WhiredPlanck
Copy link
Contributor

rime/librime#649 is to do so.

@WhiredPlanck
Copy link
Contributor Author

@hchunhui Since librime doesn’t merge the PR yet, the CI tests here will not be succeed. And you can see that rime/librime#649 also failed the CI test of docker due to a build error from librime-lua, I have no idea how to fix it ......

@hchunhui
Copy link
Owner

hchunhui commented Oct 4, 2023

@hchunhui Since librime doesn’t merge the PR yet, the CI tests here will not be succeed. And you can see that rime/librime#649 also failed the CI test of docker due to a build error from librime-lua, I have no idea how to fix it ......

I have solved the build error in master branch, please rerun CI test in rime/librime#649 .

@WhiredPlanck
Copy link
Contributor Author

@hchunhui Cool! So much thanks!

@WhiredPlanck
Copy link
Contributor Author

WhiredPlanck commented Oct 5, 2023 •
edited

@hchunhui rime/librime#649 still failed the Docker CI test due to boost filesystem not found. I think it needs the changes from here.

UPDATE: And I’m right. After I try to apply the patch of this PR (rime/librime@1b2a467 ), rime/librime#649 succeed the Docker CI test.

@hchunhui
Copy link
Owner

hchunhui commented Oct 5, 2023

@WhiredPlanck The master branch should fix the CI error.

And to ensure a smooth transition, please add proper #if guard, e.g.

#if __cplusplus >= 201703L || _MSVC_LANG >= 201703L
// std....
#else
// boost...
#endif

@WhiredPlanck
Copy link
Contributor Author

@hchunhui Will fix it soon.

@WhiredPlanck
Copy link
Contributor Author

@hchunhui Done.

@hchunhui hchunhui merged commit a0e78c4 into hchunhui:master Oct 6, 2023
4 checks passed
@hchunhui
Copy link
Owner

hchunhui commented Oct 6, 2023

Merged. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hchunhui hchunhui Awaiting requested review from hchunhui

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@WhiredPlanck @hchunhui