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

feat: Initial commit #1

Merged
merged 8 commits into from
May 31, 2020
Merged

feat: Initial commit #1

merged 8 commits into from
May 31, 2020

Conversation

mxschmitt
Copy link
Member

@mxschmitt mxschmitt commented May 9, 2020

Theres a lot missing.

I would like to have also some tests.

TODO:

  • add bundle system (not sure which one. Normal tsc should be enough. maybe https://github.com/zeit/ncc)
  • add missing types
  • adjust license, so its okay to use. Needs to be still there afaik and then our one.
  • clarify if Yarn or NPM

@mxschmitt mxschmitt requested a review from mmarkelov May 9, 2020 21:34
LICENSE Outdated Show resolved Hide resolved
@mmarkelov
Copy link
Member

@mxschmitt I'm going to start work on it. So maybe we can push some kind of init commit?)

@mxschmitt
Copy link
Member Author

@mxschmitt I'm going to start work on it. So maybe we can push some kind of init commit?)

we if its okay for you you can either push into that branch/PR or we merge this one already.

@mmarkelov
Copy link
Member

@mxschmitt ok! I'll push changes into this branch)

],
"scripts": {
"prebuild": "rm -rf lib/",
"prepublishOnly": "yarn build",
Copy link
Member

Choose a reason for hiding this comment

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

We should select yarn or npm to use. Cause right now there is yarn build command, but also there is package-lock.json file

Copy link
Member Author

Choose a reason for hiding this comment

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

yee, thats why I mentioned it in the TODOs. Im ok with both, lets maybe use npm?

Copy link
Member

Choose a reason for hiding this comment

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

@mxschmitt I suppose using npm is better, cause we are using it for jest-playwright, so we should be consistent

- Separate defaults and types
@mxschmitt
Copy link
Member Author

I think we have to mention it in the LICENSE, that his is a forked version of the jest-dev-server.

btw. what do you think about the name jest-process-manager?

@mmarkelov
Copy link
Member

I think we have to mention it in the LICENSE, that his is a forked version of the jest-dev-server.

IDK if we should mention it in LICENSE, of course we must put it in README, I think it will be enough

what do you think about the name jest-process-manager?

That's totally fine for me ;)

@mxschmitt
Copy link
Member Author

I think we can merge and release / test a version.

But for the license we have to include the old one as mentioned here: https://github.com/smooth-code/jest-puppeteer/blob/master/packages/jest-dev-server/LICENSE

@mmarkelov
Copy link
Member

I think we can merge and release / test a version.

Yeah! I think so.

We can mention them in our LICENSE, but IDK how)

@mxschmitt
Copy link
Member Author

I think we can merge and release / test a version.

Yeah! I think so.

We can mention them in our LICENSE, but IDK how)

we have to include a copy of their license and then our license (MIT).

@mmarkelov
Copy link
Member

we have to include a copy of their license and then our license (MIT).

@mxschmitt how we should include copy of their license? Like put it in ORIGINAL_LICENSE?

@mxschmitt mxschmitt merged commit 06f7086 into master May 31, 2020
@mxschmitt mxschmitt deleted the initial-version branch May 31, 2020 15:02
@mxschmitt
Copy link
Member Author

Okay, merged for now. How should we go on? Maybe fix the README and then a first release / make it public?

@mmarkelov
Copy link
Member

@mxschmitt yeah! I think we should fix the Readme and we are ready to go. Also it will be nice to ask @thernstig to open issues from jest-dev-server

@thernstig
Copy link

Added issue #3 and #4

Did not add one for argos-ci/jest-puppeteer#340 since I assume you already fixed this.

Note: I have not been able to test this package yet obviously, so it's possible you can immediately close some of the issues above if they are already considered in this new package.

@mmarkelov
Copy link
Member

@thernstig thanks a lot!

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.

None yet

3 participants