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

Test windows #356

Merged
merged 13 commits into from
Jan 25, 2021
Merged

Test windows #356

merged 13 commits into from
Jan 25, 2021

Conversation

blobbered
Copy link
Contributor

This PR is one of several smaller ones made from this original PR, by klingtnet.

This PR is responsible for cleaning up the go build and test so that they run on windows.

Andreas Linz and others added 4 commits October 15, 2020 19:24
We can safely assume that the application can be build when the test
suite succeeds.
Tests will fail if the unmmap error is returned instead of being logged
(FIXME annotation).
@blobbered blobbered force-pushed the test-windows branch 7 times, most recently from b958cc0 to e8cf816 Compare November 25, 2020 19:39
@dschott68 dschott68 self-requested a review December 16, 2020 21:13
@dschott68
Copy link
Contributor

Hi @blobbered
It looks like 9617631 included "tags" file at the top level. Could you make an update to remove that from this PR, please?

@@ -19,14 +19,28 @@ jobs:
with:
node-version: "10.x"
- run: npm install
go-test:
lint:
Copy link
Contributor

Choose a reason for hiding this comment

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

Since lint is only for ubuntu and not windows, did you want it in this PR or did you want to split it off into separate smaller PR?

@@ -563,15 +563,16 @@ func (b *bufWriter) offset() uint32 {
return uint32(off)
}

func (b *bufWriter) flush() {
func (b *bufWriter) flush() error {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we care that the other callers of flush() besides Flush() are not handling the returned error?

flush_callers

@dschott68 dschott68 merged commit 2850189 into hound-search:master Jan 25, 2021
@salemhilal salemhilal mentioned this pull request Jan 19, 2022
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