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

Minor improvement - introduce RawSeed #2312

Closed
wants to merge 3 commits into from

Conversation

lehins
Copy link
Contributor

@lehins lehins commented Jun 4, 2021

I needed a small task just to get acquainted with the codebase, so I found a few minor improvements I could make while looking through the repo. One of which was adding RawSeed instead of a 5-tuple of Word64s.
I am totally fine with discarding this PR, since it was more of an exercise to build the project and verify my new tooling and dev setup works fine. (Couple files where reformatted with ordmolu, so diff is a bit bigger than I intended)

CC @JaredCorduan

Copy link
Contributor

@nc6 nc6 left a comment

Choose a reason for hiding this comment

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

LGTM

@nc6
Copy link
Contributor

nc6 commented Jun 7, 2021

However, looks like there's some tests in various places that still need to be updated.

@lehins
Copy link
Contributor Author

lehins commented Jun 7, 2021

However, looks like there's some tests in various places that still need to be updated.

Have no idea how I missed these couple of modules. I thought I did build the whole project. In any case, I finished the update.

Copy link
Contributor

@JaredCorduan JaredCorduan left a comment

Choose a reason for hiding this comment

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

Looks great to me!

@JaredCorduan
Copy link
Contributor

@lehins buildkite isn't ever going to run for this PR since it is from a fork (which is preventing us from merging). But now you are in the organization, so you could push a branch.

@lehins
Copy link
Contributor Author

lehins commented Jun 7, 2021

Closed in favor of #2314

buildkite isn't ever going to run for this PR since it is from a fork (which is preventing us from merging)

It means that people from the community that aren't part of the organization can never contribute. That's odd for an open source project.

@lehins lehins closed this Jun 7, 2021
@JaredCorduan
Copy link
Contributor

Closed in favor of #2314

buildkite isn't ever going to run for this PR since it is from a fork (which is preventing us from merging)

It means that people from the community that aren't part of the organization can never contribute. That's odd for an open source project.

indeed, I don't like it either. I would love to find a solution. In the past, I have cherry-picked the commits from community member PRs into a branch of my own, which is not a great solution.

@JaredCorduan
Copy link
Contributor

@michaelpj had a great suggestion, IMHO: buildkite/feedback#288 (comment)

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.

None yet

3 participants