Skip to content
This repository has been archived by the owner on Dec 10, 2020. It is now read-only.

Migrate browser folder files to typescript & fix npm build #155

Merged
merged 4 commits into from
Oct 23, 2020

Conversation

cgewecke
Copy link
Contributor

@cgewecke cgewecke commented Oct 21, 2020

#148

NB: PR targets #153

PR

RlpxPeer depends on two Node platform modules which aren't available in the browser: net and dgram. These probably need to be resolved to some alias.

Have configured webpack to ignore that problem for the moment so npm run build runs in some fashion for the monorepo transition.

#108 advises that we add a basic test to check the browser build, perhaps we can address whether or not the bundle is valid as part of that.

Base automatically changed from upgrade/eslint-config to master October 23, 2020 09:10
@coveralls
Copy link

coveralls commented Oct 23, 2020

Coverage Status

Coverage increased (+0.1%) to 76.297% when pulling dcf1dab on fix/browser-build into f774a4b on master.

@holgerd77
Copy link
Member

Side question for my understanding (and since I would find it pretty interesting to apply as well: does this "NB: PR targets..." trigger this "Base automatically changed" message or does this always happen if one targets another branch than master and the branch gets merged? 🤔

Haven't worked in such a branch cascade before but am constantly battling with similar situations...

Copy link
Member

@holgerd77 holgerd77 left a comment

Choose a reason for hiding this comment

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

Looks good.

@@ -21,6 +21,7 @@ jobs:
- uses: actions/checkout@v2
- run: npm install
- run: npm run lint
- run: npm run build
Copy link
Member

Choose a reason for hiding this comment

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

Oh yeah, super useful

@holgerd77 holgerd77 merged commit 1abcd26 into master Oct 23, 2020
@holgerd77 holgerd77 deleted the fix/browser-build branch October 23, 2020 09:18
@cgewecke
Copy link
Contributor Author

@holgerd77

Side question for my understanding (and since I would find it pretty interesting to apply as well: does this "NB: PR targets..." trigger this "Base automatically changed" message or does this always happen if one targets another branch than master and the branch gets merged?

Good question! - it's definitely new right?

Just made the notes for clarity and think this must be the correct explanation:

if one targets another branch than master and the branch gets merged

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants