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

Don't check in built content #643

Closed
wants to merge 2 commits into from
Closed

Don't check in built content #643

wants to merge 2 commits into from

Conversation

EzraBrooks
Copy link
Contributor

Public API Changes

None

Description

Removes the built content from the repository to avoid accidental creation of giant, hard-to-resolve diffs in PRs.

WIP/TODO: how to handle examples? should these require building? can we inject a browser require polyfill (if we're not already) to make the raw src work fine?

@MatthijsBurgh
Copy link
Contributor

The only wat I accept such a change is, when we attach builds to the releases and/or release branches.

@@ -55,7 +55,7 @@
"scripts": {
"build": "grunt build",
"doc": "grunt doc",
"test": "grunt test",
"test": "npm run build && grunt test",
Copy link
Contributor

Choose a reason for hiding this comment

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

Not accepting this change. When you make your case I could accept grunt build && grunt test

Copy link
Contributor Author

Choose a reason for hiding this comment

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

npm run build here guarantees that if a user changes the build script, the changes are reflected here. Is your concern that it's NPM-specific? It seemed from the README that this project is developed with NPM.

@EzraBrooks
Copy link
Contributor Author

The only wat I accept such a change is, when we attach builds to the releases and/or release branches.

Can you explain to me why the built content is checked into the repo? This is extremely atypical in my experience. The JavaScript ecosystem has package managers that work, we don't need to ship tarballs.

@MatthijsBurgh MatthijsBurgh added the v2 Issues and PRs, which will be fixed in v2, as it is a breaking change label Nov 28, 2023
@EzraBrooks
Copy link
Contributor Author

Superseded by #655

@EzraBrooks EzraBrooks closed this Dec 14, 2023
@EzraBrooks EzraBrooks added this to the 2.0.0 milestone Jun 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v2 Issues and PRs, which will be fixed in v2, as it is a breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants