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

Merge performance improvements into upstream #26

Open
richardartoul opened this issue Jan 4, 2019 · 7 comments
Open

Merge performance improvements into upstream #26

richardartoul opened this issue Jan 4, 2019 · 7 comments

Comments

@richardartoul
Copy link

Hello!

My team has been maintaining a forked branch of vellum with some performance pooling improvements. These changes have significantly reduced memory allocations and memory consumption of the FST building process in our timeseries database.

I know a previous teammate of mine (who is unfortunately on leave of absence) had begun contributing some code back to upstream. Are you open to me opening P.Rs to contribute the remaining performance improvements back? We'd love to not maintain our own fork, as well as contribute back to the library that saved us a ton of work :)

Thanks again for open sourcing such a great library.

@abhinavdangeti
Copy link
Member

Absolutely, we welcome all valuable contributions. Here's instructions on how you could contribute back to the library ..

https://github.com/couchbase/vellum/blob/master/CONTRIBUTING.md

@richardartoul
Copy link
Author

richardartoul commented Jan 5, 2019

Great! Here is the first P.R #27

Please pardon my ignorance, but I'm having trouble signing the CLA. I went to here but couldn't figure out where to go from there.

Also C.I is failing, but I don't think its anything related to my build. Appears to be having trouble installing the errcheck linter

@richardartoul
Copy link
Author

Second P.R: #28 (rebased off the previous one to avoid conflicts, we can try and merge the other one first then this one)

Also I believe you can close #22 as its redundant with #28

@richardartoul
Copy link
Author

@abhinavdangeti Just pinging to see if you got a chance to review!

@abhinavdangeti
Copy link
Member

Hey @richardartoul, thanks for the contributions. Haven't had a chance to look into them yet. One of us here on the team will take a look as soon as we are able.

@richardartoul
Copy link
Author

Sounds good, thanks!

@richardartoul
Copy link
Author

Sorry for the spam, just wondering if you'd had a chance to take a look? The performance improvements are quite substantial and we hate to maintain our own fork

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

No branches or pull requests

2 participants