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 to upstream #1

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

Merge to upstream #1

wants to merge 701 commits into from

Conversation

werenall
Copy link

This PR merges day8/re-frame's HEAD of the master branch.

superstructor and others added 30 commits August 25, 2020 10:48
Too much risk of breakage in a future mkdocs release due to level of
customisation. Risk vs benefit etc.

This reverts commit 6996500.
@werenall
Copy link
Author

Hi there!
Here in Magnet we've been leveraging your work on SSR and decided to contribute back by merging your fork to the upstream.
By the way, have you considered opening a PR to the original re-frame?

@werenall
Copy link
Author

Huh, travis is failing because of the lein karma-once. But so it would fail in day8/re-frame. But apparently they have turned travis off https://travis-ci.org/github/Day8/re-frame

@harold
Copy link

harold commented Aug 26, 2021

Hi! 👋 @werenall - Thanks for this.

In our deployed applications with ssr, we have been running with the tip of this repo- and though it's been a few years, I'm not sure what upstream changes would benefit us (though, I'd bet there are some!).

I believe we looked into getting this merged when the improvements were originally made (@charlesg3 would know more/remember better), but iirc there wasn't much understanding upstream of the benefits (for both test and ssr) of these improvements to the symmetry of clj/cljs.

Very happy to know you've gotten some benefit from this effort.

There are hundreds of forks of re-frame, no harm in you forking this one and moving forward on your own, if that turns out to be best.

Regardless, hope you have a great day.

@charlesg3
Copy link

I'm all for seeing if we can get this merged into original re-frame -- seems only sensible to try to get clj/cljs working together.

@werenall
Copy link
Author

Cool! So what can I do to help?

@harold
Copy link

harold commented Aug 30, 2021

Cool! So what can I do to help?

Would getting your fork merged into the mainline re-frame repo make sense?

@werenall
Copy link
Author

Sure. I'll make sure to @ you guys however as it's you who did all the heavy lifting. I just did a rebase 😅

@werenall
Copy link
Author

Alright, I created this PR. I squashed the changes to avoid the noise related to your internal s3wagon config. The following week I'll be busy with other project but I'll try my best to be active on this PR.

@harold
Copy link

harold commented Aug 31, 2021

Cool! 😎

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.