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

Finding a way to upstream a stream of changes from my fork #54

Open
jbhoot opened this issue Nov 12, 2022 · 6 comments
Open

Finding a way to upstream a stream of changes from my fork #54

jbhoot opened this issue Nov 12, 2022 · 6 comments

Comments

@jbhoot
Copy link

jbhoot commented Nov 12, 2022

I have made several modifications to this repo in my (new) fork, like

  • fix onCheck event handler (jbhoot@1c5beaf)
  • fix how role attribute is created: as an attribute instead of as a property (jbhoot@c8c238c)
  • fix how boolean attributes are created (jbhoot@33890ed)
  • add onLinkClick handler (which also handle preventDefault()) (jbhoot@79c1c4f)
  • add new attributes

Initially I thought of creating a branch and an associated pull request for each change. However, I need to use these changes immediately in my code. So I need all of those changes within a single branch.

Now I am committing everything to the main branch of my fork. Each commit still deals with only one thing. Each commit message also describes the why, if needed.

This way, we can merge these changes in this repo at leisure.

Let me know if and how you would like to handle this.

@pbiggar
Copy link
Member

pbiggar commented Nov 12, 2022

What I've been doing is upstreaming individually, and then waiting until they're all in toe switch over. Another option would be rebasing the fork as they get merged.

Would this work for you?

onCheck, role, boolean attrs, and new attrs seem great. I'm not sure what onLinkCheck does so I'm not sure yet.

@jbhoot
Copy link
Author

jbhoot commented Nov 12, 2022

What I've been doing is upstreaming individually, and then waiting until they're all in toe switch over. Another option would be rebasing the fork as they get merged.

Creating a PR per change won't work at the moment. I need to progress in my work at a prolific pace. Hence I need to keep an up-to-date repo with all the fixes for the bugs I encounter in rescript-tea.

But I cannot expect you to accept every PR I make, and that too immediately.

So rebasing the fork, or may be cherry-picking commits, should work.

@jbhoot
Copy link
Author

jbhoot commented Nov 12, 2022

On the other hand, feel free to pick anything from my fork without waiting for me to create PRs.

@jbhoot
Copy link
Author

jbhoot commented Nov 12, 2022

I'm not sure what onLinkCheck does so I'm not sure yet.

Would

a(list{href("#"), onClick(v => LinkClicked(v))}, list{"Click me"->text})

trigger e.preventDefault() automatically?

As far as I can recall, it doesn't, causing the page to reload to the href.

a(list{href("#"), onLinkClick(v => LinkClicked(v))}, list{"Click me"->text})

would trigger preventDefault(), and issue the msg. Internally, onLinkClick uses preventDefaultOn.

So it essentially a utility function.

@pbiggar
Copy link
Member

pbiggar commented Nov 13, 2022

I see, that makes sense. Maybe it would be a good idea to include an @ocaml.doc( documentation string that explain this.

@pbiggar
Copy link
Member

pbiggar commented Nov 13, 2022

So rebasing the fork, or may be cherry-picking commits, should work.

Great yeah. If you make PRs for anything that's ready and rebase off them, we should be able to get these done quickly! Thanks again!

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