Skip to content

Conversation

@jcelerier
Copy link
Contributor

For #28

Copy link
Collaborator

@BurningEnlightenment BurningEnlightenment left a comment

Choose a reason for hiding this comment

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

I found some minor nits, but we'll need to wait for Niall's approval.

@ned14
Copy link
Owner

ned14 commented Jun 23, 2022

I pretty much agree with @BurningEnlightenment's feedback. Thanks for the PR!

@ned14
Copy link
Owner

ned14 commented Jun 29, 2022

@BurningEnlightenment Will this need some changes to the vcpkg port?

@ned14 ned14 merged commit e5414b7 into ned14:master Jun 29, 2022
@github-actions
Copy link

Unit Test Results

    3 files  -  1    41 suites  -15   6m 56s ⏱️ - 2m 40s
  49 tests ±  0    49 ✔️ ±  0  0 💤 ±0  0 ✖️ ±0 
145 runs  -51  145 ✔️ -51  0 💤 ±0  0 ✖️ ±0 

results for commit e5414b7 ± comparison against base commit 568e181

@BurningEnlightenment
Copy link
Collaborator

Yeah, I'll take care of that. Will probably take a few days due to the review process, etc.

@jcelerier
Copy link
Contributor Author

thanks ! :)

BurningEnlightenment added a commit that referenced this pull request Jul 4, 2022
@BurningEnlightenment
Copy link
Collaborator

I fixed a minor build system issue and opened microsoft/vcpkg#25560

@ned14
Copy link
Owner

ned14 commented Jul 6, 2022

@BurningEnlightenment
Copy link
Collaborator

I will take a look in an hour or so.

@BurningEnlightenment
Copy link
Collaborator

BurningEnlightenment commented Jul 6, 2022

https://github.com/ned14/outcome/runs/7141879761?check_suite_focus=true suggests this PR broke Outcome CI?

This looks like an issue I already encountered while updating the vcpkg port (not #32). I pushed 9cdcd45 onto the master branch to fix it. So the CI should be green again next time.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants