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 CI for linux/arm64, darwin/arm64 #528

Merged
merged 2 commits into from Oct 17, 2022
Merged

Add CI for linux/arm64, darwin/arm64 #528

merged 2 commits into from Oct 17, 2022

Conversation

arp242
Copy link
Member

@arp242 arp242 commented Oct 16, 2022

Use CircleCI to add CI for this.

Fixes #136

Use CircleCI to add CI for this.
Provide full API compatibility on unsupported systems such as AIX.
@arp242 arp242 merged commit 666d503 into main Oct 17, 2022
@arp242 arp242 deleted the circleci-project-setup branch October 17, 2022 01:57
@sagikazarmark
Copy link

Do you have an ETA for the next release? This PR in particular would help remove some wrapper code around fsnotify from Viper to let it build on unsupported platforms.

@arp242
Copy link
Member Author

arp242 commented Nov 3, 2022

Do you have an ETA for the next release? This PR in particular would help remove some wrapper code around fsnotify from Viper to let it build on unsupported platforms.

Not really; something on the order of months.

If the wrapper code already exists and works, then I don't think we really need to rush out a release?

@arp242
Copy link
Member Author

arp242 commented Nov 3, 2022

Also, looking at the workaround, it seems you added it in 2021 @sagikazarmark over here: https://github.com/spf13/viper/blob/master/watch_wasm.go

Don't be afraid to report these kind of things upstream (i.e. here); the fix is trivial, and it almost certainly would have been in the v1.6.0 release ... if I had known about it. I just happened to stumble on it in this PR, which is why it's fixed here.

Doing a release now with just some minor bugfixes just creates a lot of churn for everyone IMO. The workaround doesn't seem so bad to keep for the time being.

@sagikazarmark
Copy link

No rush on removing the wrapper now. Just wanted to know if a release would happen in the near future. I would have kept the relevant PR open.

TBH I completely forgot about it. The only reason it came to my attention is because someone asked for adding AIX to the list in addition to JS.

Thanks for all the work you are doing on fsnotify!

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.

Builders for continuous integration
2 participants