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

[Core] Improve import path for published lib #3921

Merged
merged 1 commit into from
Apr 11, 2016

Conversation

nathanmarks
Copy link
Member

This addresses #2679 by copying a minimal package.json along with the README.md, CHANGELOG.md and LICENSE to the build folder after the JS files have been babel'd.

I added a script to copy the files and put it under ./scripts/, I also moved the svg-icon index generator script there.

I haven't added any tests to confirm the output from npm run build, but can definitely do so if this is the setup we move forward with.

Let me know if there's anything we need to move over that I forgot...!

  • PR has tests / docs demo, and is linted.
  • Commit and PR titles begin with [ComponentName], and are in imperative form: "[Component] Fix leaky abstraction".
  • Description explains the issue / use-case resolved, and auto-closes the related issue(s) (http://tr.im/vFqem).

"coverage": "npm run test",
"coverage:combine": "istanbul report --dir test/coverage/combined --include test/**/*coverage.json text-summary lcovonly json",
"lint": "eslint src docs/src test/browser test/unit && echo \"eslint: no lint errors\"",
"prebuild": "rimraf lib",
"prepublish": "in-publish && npm run build || not-in-publish",
"prebuild": "npm run clean:build",
Copy link
Contributor

Choose a reason for hiding this comment

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

Noticed the cleaning is also in the build "script"

@nathanmarks nathanmarks added PR: needs revision The pull request can't be merged. More details is available on the code review or fails in the CI and removed PR: Needs Review labels Apr 10, 2016
@nathanmarks nathanmarks changed the title [Core] Improve import path for published lib [WIP][Core] Improve import path for published lib Apr 10, 2016
@nathanmarks nathanmarks added PR: Needs Review and removed PR: needs revision The pull request can't be merged. More details is available on the code review or fails in the CI labels Apr 10, 2016
@nathanmarks nathanmarks changed the title [WIP][Core] Improve import path for published lib [Core] Improve import path for published lib Apr 10, 2016
"keywords": [
"react",
"react-component",
"material design",
"material-ui",
"material ui"
Copy link
Member

Choose a reason for hiding this comment

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

I think that was there to aide search in npmjs for "material ui", but maybe it works anyway with the hyphenated version?

Copy link
Member

Choose a reason for hiding this comment

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

As far as I have used the npm search tool, I have notices that every character matters. Especialy a -. I think that it's better to keep both keywords.

Copy link
Contributor

Choose a reason for hiding this comment

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

Woops, I thought this was a duplicate when I first read it, I didn't even see the - difference.

Copy link
Member Author

Choose a reason for hiding this comment

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

Let's just be safe and go with @oliviertassinari 's findings.

@mbrookes
Copy link
Member

@nathanmarks - this looks great!

@nathanmarks
Copy link
Member Author

@mbrookes done that thing!

@nathanmarks
Copy link
Member Author

@mbrookes should I squash this now?

@newoga
Copy link
Contributor

newoga commented Apr 11, 2016

@mbrookes should I squash this now?

This looks good, let's squash and merge @nathanmarks.

@nathanmarks
Copy link
Member Author

@newoga we'll need to update the README.md before the actual release right? Or somehow make the change explicit next time @hai-cea cuts a new release. I just reviewed @oliviertassinari 's codemod so we can get that on track for 0.15.0 as well.

@newoga
Copy link
Contributor

newoga commented Apr 11, 2016

@newoga we'll need to update the README.md before the actual release right?

Yes, in the past @alitaheri has graciously taken on the task for preparing releases, including updating README, changelog, release notes, etc. We'll make sure to do that before official release and publish, I think this is good for now. And I agree we should try to get in @oliviertassinari's codemod before releasing too.

Improves the import path by creating a package.json
inside the build directory so the library can be
published with a flattened root folder. The lib root
package.json is transformed into a minimal version and
the README, CHANGELOG and LICENSE are copied over.

Resolves: mui#2679
@nathanmarks
Copy link
Member Author

Squashed it

@alitaheri
Copy link
Member

@newoga I'm still around, just a bit busy recently 😅 😅. I can take care of the release if you guys are ok with it, it's no trouble 😁

@newoga
Copy link
Contributor

newoga commented Apr 11, 2016

@newoga I'm still around, just a bit busy recently 😅 😅. I can take care of the release if you guys are ok with it, it's no trouble 😁

That's what I like to hear @alitaheri, you do a great job with it so that would be awesome! 👍 🎉 (And I've been busy too recently, no worries! 😄 )

@alitaheri
Copy link
Member

😄 Alright, I'll take care of it as before. Just ping me when it's good to go 😁

@oliviertassinari
Copy link
Member

@alitaheri Awesome, thanks!
Good luck in advance, we have merged a lot of PR 😁.

@alitaheri
Copy link
Member

Yeah, 😆 Thanks 😁 😁

@nathanmarks
Copy link
Member Author

@newoga @oliviertassinari

Anything else needed to get this merged?

@newoga
Copy link
Contributor

newoga commented Apr 11, 2016

I think this is ready!

@newoga newoga merged commit 730d31a into mui:master Apr 11, 2016
@zannager zannager added the core Infrastructure work going on behind the scenes label Mar 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Infrastructure work going on behind the scenes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants