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

bug: two implementation details are fragile in partition-view #550

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

Plictox
Copy link
Collaborator

@Plictox Plictox commented May 5, 2023

  • Kind: bugfix

Description

Checklist

Note to maintainers

  • Read this wiki page.
  • Make sure the PR has a milestone.
  • Assign yourself before merging.
  • Either do a regular merge:
    • for PRs containing several commits following conventional-commits,
    • or for PRs containing 1 commit shared with a later PR (to preserve the SHA1)
  • Or do a squash-merge:
    • for PRs containing only 1 commit (not shared with a later PR),
    • or for PRs containing several commits that need not be kept in the history;
    • Update the commit message header with a conventional-commit type,
    • Add a footer Close #… if a related issue exists.

@Plictox Plictox self-assigned this May 5, 2023
@erikmd
Copy link
Member

erikmd commented May 5, 2023

Thanks @Plictox!

Note BTW: the standard convention for assignees in GitHub public repositories is generally as follows:

→ The developer that implements the feature (you) is the assignee of the issue (#456)

→ The maintainer that reviews the contribution (me) is the assignee of the PR (#550) : updating this now.

Have a nice WE and see you.

@erikmd erikmd assigned erikmd and unassigned Plictox May 5, 2023
…-partition-requests

* origin/bug-partition-requests:
  bug: passing eid as an argument for Partition requests
Copy link
Member

@erikmd erikmd left a comment

Choose a reason for hiding this comment

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

Thanks! some first suggestions

src/state/learnocaml_api.ml Outdated Show resolved Hide resolved
src/state/learnocaml_api.ml Outdated Show resolved Hide resolved
src/state/learnocaml_api.ml Outdated Show resolved Hide resolved
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.

Bug(partition-view): two implementation details are fragile
2 participants