Skip to content

Conversation

@Lezek123
Copy link
Contributor

@Lezek123 Lezek123 commented Jul 31, 2020

Related issues: #1033 and #1034

After trying out a few approaches I've choosen the following way of upgrading polkadot-js/apps packages in Pioneer (this process shares some resemblances with #469):

Part 1: The merge

  1. In current monorepo (iznik branch): I created pioneer-new-apps branch on which I reduced the number of Pioneer packages to the required minimum set (Pioneer upgrade step 1: Find minimum set of required packages #1033) and then moved all remaning old polkadot-js packages to another folder (old-apps) to avoid merge conflicts, but still have them easily accessible for a quick comparasion if needed (59d657c)
  2. Added polkadot-apps as git remote locally (git remote add polkadot-apps https://github.com/polkadot-js/apps.git)
  3. Fetched v0.51.1 of the apps to a new local branch (git fetch polkadot-apps refs/tags/v0.51.1:refs/heads/polkadot-new-apps)
  4. git checkout polkadot-new-apps
  5. Moved everything to /pioneer subfolder: mkdir pioneer and ls -a1 | grep -v ^pioneer | xargs -I{} git mv {} pioneer
  6. git commit -m "Move to subfolder" (4fb6768)
  7. git checkout pioneer-new-apps
  8. git merge -Srecursive -Xno-renames polkadot-new-apps (prevent detecting renames now that we moved all the old apps away)
  9. The only conflicts that remained were files directly in the /pioneer folder (so mostly config files for babel, jest, TS, linter, docker etc., README.md and of course package.json).
  10. Originally I then processed to resolve some of the conflicts before commiting the merge and then made everything work in a few following commits, but since that started to look a little chaotic I decided to clean up the branch: I went back to step 8, but this time performed the mege with --strategy-option=ours (0d5ea9e) and then merged the work from previosly adjusted branch with git merge --squash -s resolve. This allowed use to have just one commit that represents all the adjustments that were implemented following that "simplified merge" , so it should be now be much easier to manage and review (b29a355).

Part 2: Monorepo setup

Related commit: b29a355

I made all the necessary adjustments required for the new apps build, lint and start scripts to work in our monorepo setup.

This of course required temporary excluding joy- and old-apps packages from beeing considered, so I:

  • added them in "exclude" section of pioneer/tsconfig.json
  • added .skip-build files, which are recognized by polkadot's build script
  • added them to .eslintignore

Once I managed to get the new apps working as a monorepo workspace I actually decided to temporary exclude them from the root monorepo workspaces to prevent the new dependencies from "messing up" other projects (CLI, network tests, @joystream/types etc.). I also modified Pioneer's CI checks a little bit, so that they correctly work in the current setup and I can take advantage of them when adjusting the packages before we get to the stage of upgrading the dependencies at the root level of the monorepo.

Some adjustments that were made so far:

  • I accepted the new versions of some of the configuration files which I knew could affect the build and some features of the new polkadot-js/apps (ie. babel.config.js, i18next-scanner.config.js, .eslintrc.js).
  • polkadot-js/apps uses .yarn/releases and .yarn/plugins, but this didn't seem to work inside our monorepo. Perhaps there was a better way to handle this, but removing those folders along with .yarnrc was the quickfix I decided to implement for now.
  • Had to add some explicit dependencies in Pioneer's package.json like react-i18next (which didn't get hoisted from packages/react-components) and a newer version of typescript (which is required to build the new apps).
  • Had to override polkadot-dev-build-ts binary with our own script (b29a355#diff-23cc9ad2bdc8b3cb3418d168e9d2b0f2), since the original one had some issues with the process working directory (which I further explained in the linked file)
  • Temporary turned off header/header rule as it was causing some issues (and will probably cause more once we start re-activating joy- packages)
  • Adjusted Pioneer's CI checks to match the new, temporary structure of the repo
  • Overriden lockfile in /pioneer with the one generated by our version of yarn
  • Added missing madge as devDependency to @joystream/types

Tooling:
It looks like we're not using a lot of the features that the original repository provides, so I excluded some yarn scripts etc. during the merge, but we could later consider including and adjusting them (ie. there seems to be some setup in place that allows building the original repository as Electron app and also this: https://github.com/polkadot-js/apps#ipfs).

I included the changes from #1071 in this PR so currently marking it as draft until those are merged.

actions-user and others added 30 commits July 4, 2020 20:02
* Modal for setting sub identities

* Small update

* Add/Remove rows for subs

* Disable on invalid raw value

* Set of actual identities

* Rewrite yarn.lock
* 0.48

* CHANGELOG
* Indicate own nominees on targets

* icon prop to Badge

* AddressName cachin cleanups
* Handlers for OpaqueCall input/display types

* Simplify disabled OpaqueCall
* Add unit tests for electron account management

* Use tmp package in file store tests.

* Enable @typescript-eslint/no-useless-constructor extension eslint rule.

* Explicitly assign private field in constructor.

Co-authored-by: Krzysztof Jelski <[email protected]>
* Adjust AccountName icon alignments

* Simplify

* 1px for whatever reason

* EditButton sizing fix
* Display accountIndex on SideBar (as available)

* accountIndex cache

* Cleanup accountIndex re-renders

* Unneeded accountIndex effect dep
* Society generator

* Modal popup

* Log accountId & hex

* Spacing

* Reduce for addressToBits
* Name query filter on targets

* s/name/filter

* Apply ?filter=<string> on all Filtered wrappers
* add tab and query

* display accounts amount and conviction

* delegation modal without react-component

* add conviction dropdown

* number not string

* use string rather than accountId and KeyringAddress

* all changing

* use ConvictionDropdown from react-components

* delegate column in accounts

* with expander

* with modals

* ordering

* Apply suggestions from code review

Co-authored-by: Jaco Greeff <[email protected]>

* ordering

* implement suggestion and fix lint

* Small layout adjustments with help text

* Only delegate to owned accounts

* Use AddressMini for delegation layout

* Remove ununsed imports

Co-authored-by: Jaco Greeff <[email protected]>
actions-user and others added 10 commits July 26, 2020 16:07
@Lezek123 Lezek123 marked this pull request as draft July 31, 2020 12:15
@Lezek123 Lezek123 requested a review from mnaamani July 31, 2020 12:35
@Lezek123
Copy link
Contributor Author

Lezek123 commented Aug 11, 2020

New changes:

  • Merged current iznik branch
  • Configured Pioneer as workspace, since resolutions have already been moved to root package.json during @joystream/types upgrade
  • Moved joy-utils to joy-utils-old, since I'm going to partially re-enable this package while reactivating and upgrading other depependent packages like joy-members, joy-election etc. I also plan to modify the directory stucture during this process to match the new joystream-js sketch, allowing for better understanding of the content of this package and opening a possibility to move some of its reusable functionality out of Pioneer. I decided to include this move (joy-utils => joy-utils-old - 17b03bb) as part of this PR in order to make subsequent PRs as clean and easy to review as possible.

Linter

As part of configuring the new Pioneer to work as a monorepo workspace I had to globally bump the eslint version and the versions of some plugins. This allows the current Pioneer to pass the linting, but also means that other projects will require some minor adjustments. I tried to temporary separate eslint-related dependencies completely between Pioneer and other projects, but it turned out hard to achieve and I believe we planned to update our eslint dependencies in the monorepo anyway, so perhaps this not too bad of a time to do this.

Most of the related work is in this commit: bf24ab0

Merge conflicts and future upgrades

I discovered a way to get a cleaner list of conflicts during the upgrade-from-upstream merge. We can achieve this by using: git merge -s recursive -Xsubtree=pioneer [UPSTREAM-BRANCH], where UPSTREAM-BRANCH is a branch from polkadot-apps/js repository that we wish to use a a base for the upgrade.

This still doesn't give us a perfect list of conclicts, ie. the changes in app-accounts or app-staking seem to be so major that git isn't able to correctly map them no matter which strategy we choose, but in general this was very helpful and provided me some very useful information, some of which I will use when creating new PRs.

I tested this approach with an example upgrade of polkadot-js/apps from v0.51.1 to the currently newest version: v0.53.1 and there were only some minor conflicts in files like package.json or yarn.lock. The list of changes also seemed very clean and easy to read. I'll try to avoid modifying the core apps as much as possible to allow very easy upgrades from upstream in the future.

Re-activating joy- packages

I was able to make joy-members package work locally in the setup introduced in this PR (and runtime from #1126), but I'm going to include this work in my next PR, as I didn't want to expand the scope of this one too much.

@Lezek123 Lezek123 marked this pull request as ready for review August 11, 2020 10:42
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.

9 participants