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 with facebook/create-react-app/master #8

Closed
wants to merge 83 commits into from

Conversation

calebmshafer
Copy link
Member

This the first time we're doing a merge from upstream so I wanted to layout the process I followed and I'll add it to our README if we all agree it's the right way to go.

  1. Push upstream master to imodeljs master, there should never be any conflicts. (needs to be pushed by an admin but we should automate this at some point...)
    git remote add upstream https://github.com/facebook/create-react-app.git
    git fetch upstream
    git checkout master
    git merge upstream/master
    git push
  2. Update imodeljs branch with new imodeljs master
    a.
    git checkout imodeljs
    git checkout -b merge-with-master
    git merge origin/master
    b. Resolve merge conflicts
    c. Set the version of react-scripts to -dev.0 of whatever is incoming from master
    c. Commit, push and open PR

The main reason I wanted to create a PR branch is to have a linear first-parent history for all explicit changes to our branch instead of directly pushing overtop of them all. That way it's an explicit merge commit instead of a fast-forward merge. Let me know what you think.

MichaelDeBoey and others added 30 commits March 24, 2020 09:10
Co-authored-by: Ian Schmitz <ianschmitz@gmail.com>
Bumps [acorn](https://github.com/acornjs/acorn) from 6.4.0 to 6.4.1.
- [Release notes](https://github.com/acornjs/acorn/releases)
- [Commits](acornjs/acorn@6.4.0...6.4.1)

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Passing an array with a single entry is not equivalent. This causes Webpack
to generate another wrapper module around the entry. This is just
unnecessary overhead and bytes.
Co-authored-by: Ian Schmitz <ianschmitz@gmail.com>
Co-authored-by: Ian Schmitz <ianschmitz@gmail.com>
@calebmshafer calebmshafer changed the title Merge master Merge with facebook/create-react-app/master Aug 16, 2020
@CLAassistant
Copy link

CLAassistant commented Aug 16, 2020

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 55 committers have signed the CLA.

✅ calebmshafer
❌ braedongough
❌ MichaelDeBoey
❌ ai
❌ Dremora
❌ challet
❌ MostafaNawara
❌ mrmckeb
❌ charrondev
❌ iddan
❌ coryhouse
❌ sebmarkbage
❌ gaearon
❌ ianschmitz
❌ NMinhNguyen
❌ M165437
❌ StenAL
❌ alexkrolick
❌ MKorostoff
❌ hieuxlu
❌ shakib609
❌ jeremywadsack
❌ skovhus
❌ gnapse
❌ maazadeeb
❌ tanhauhau
❌ iansu
❌ nickmccurdy
❌ chybisov
❌ evankennedy
❌ samccone
❌ mhassan1
❌ favna
❌ housseindjirdeh
❌ atlanteh
❌ Timer
❌ sonicdoe
❌ availchet
❌ josenriagu
❌ burkeholland
❌ Rafael Quijada
❌ sakit0
❌ BMorearty
❌ klinem
❌ andycanderson
❌ pmmmwh
❌ jeffposnick
❌ merelinguist
❌ anuraaga
❌ ljosberinn
❌ JLHwung
❌ Lapz
❌ SimenB
❌ staff0rd
❌ chenxsan


Rafael Quijada seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@aruniverse
Copy link
Member

I think the steps you've defined above is accurate and is what i was doing earlier on. We can probably set a chron job for pushing updates from upstream to master. How are you going to get around the issue above with the CLA? Do we want to setup a release pipeline for the dev builds of this before it gets merged in?

@calebmshafer calebmshafer mentioned this pull request Aug 17, 2020
@calebmshafer
Copy link
Member Author

calebmshafer commented Aug 17, 2020

I think the steps you've defined above is accurate and is what i was doing earlier on. We can probably set a chron job for pushing updates from upstream to master. How are you going to get around the issue above with the CLA? Do we want to setup a release pipeline for the dev builds of this before it gets merged in?

@aruniverse, I disabled CLA Assistant for this entire repo so that issue should be resolved for all future PRs. In terms of a dev release, yes I'd like to get one out so that we can start testing it. Let's wait to merge this into our branch until we're ready though, I guess that means releasing the -dev version from this PR. I'll get around to that after #9.

Also, at @wgoehrig's suggestion I'll probably update those steps to require updating master to the latest react-scripts git tag in the facebook/create-react-scripts repo. Still haven't figured out where they released the package from since it does show up on npmjs but just not directly on master.

@aruniverse
Copy link
Member

Also, at @wgoehrig's suggestion I'll probably update those steps to require updating master to the latest react-scripts git tag in the facebook/create-react-scripts repo. Still haven't figured out where they released the package from since it does show up on npmjs but just not directly on master.

I didnt see an issue or pr branch where they did the release off of. Dan abramov mightve just done it himself locally. facebook#9470

@calebmshafer
Copy link
Member Author

Closing this in favor of #22

@calebmshafer calebmshafer deleted the merge-master branch October 24, 2020 17:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.