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

Build: remove browserify script #466

Merged
merged 1 commit into from Feb 11, 2021
Merged

Conversation

mdjermanovic
Copy link
Member

Removes browserify script.

It doesn't look like we're publishing the bundle it creates, or using the script/bundle for anything, so it seems there is no point in maintaining the script.

Note: npm run sync-docs updated docs/README.md with some unrelated changes that were not synced before.

@nzakas
Copy link
Member

nzakas commented Jan 28, 2021

Just to double check, can you verify that this isn’t being used by the main repo when we do a release?

@mdjermanovic
Copy link
Member Author

Just to double check, can you verify that this isn’t being used by the main repo when we do a release?

I'm pretty sure we're not using this in eslint/espree, eslint/eslint and eslint/website repos, though wouldn't mind if someone else could also verify that.

The browserify script runs Makefile.js to create build/espree.js.

Neither of Makefile.js and build/espree.js is among the files we publish to npm registry:

espree/package.json

Lines 8 to 11 in 3b4ca9e

"files": [
"lib",
"espree.js"
],

Our ESLint demo is here. The website build first installs dependencies of the ESLint demo from npm. The only direct dependency is "eslint": "latest". Espree will be installed from npm as a dependency of ESLint, without Makefile.js and build/espree.js files as they were not published.

website build then runs webpack to make demo.js, and webpack bundles Espree like all other dependencies.

The main eslint/eslint repo also uses webpack to test Linter in browsers.

As far as I'm aware, we don't have any Espree demos anymore, there's only ESLint demo.

Copy link
Member

@nzakas nzakas left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@btmills btmills left a comment

Choose a reason for hiding this comment

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

LGTM. Since we now use astexplorer for parser demo purposes, I also verified astexplorer uses the published package as well, so this shouldn't accidentally break that.

@mdjermanovic mdjermanovic merged commit 1a8ec00 into master Feb 11, 2021
@mdjermanovic mdjermanovic deleted the remove-browserify-script branch February 11, 2021 22:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants