Skip to content
This repository has been archived by the owner on Jun 29, 2021. It is now read-only.

Fix issues on #330 #331

Closed
wants to merge 2 commits into from
Closed

Fix issues on #330 #331

wants to merge 2 commits into from

Conversation

naorzr
Copy link

@naorzr naorzr commented Jul 14, 2019

No description provided.

@hasezoey
Copy link
Contributor

when you change something, please dont make always a new Pull Request, thanks

@hasezoey
Copy link
Contributor

hasezoey commented Jul 14, 2019

Note to current Maintainers, current tests only fail because of npm audit which is fixed in #327

@naorzr you didnt need todo 6e6821f, but ok

@naorzr
Copy link
Author

naorzr commented Jul 14, 2019

@hasezoey noted

@coveralls
Copy link

Coverage Status

Coverage increased (+0.4%) to 84.777% when pulling 6e6821f on naorzr:master into 943c589 on szokodiakos:master.

@coveralls
Copy link

coveralls commented Jul 14, 2019

Coverage Status

Coverage increased (+0.4%) to 84.777% when pulling 06d43c2 on naorzr:master into 943c589 on szokodiakos:master.

@hasezoey
Copy link
Contributor

@naorzr, is there currently a special test for it?, when not its ok

Copy link
Contributor

@hasezoey hasezoey left a comment

Choose a reason for hiding this comment

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

these changes are minor and @Ben305 can ignore it, when not wanted

(when no jsdoc(/tsdoc) is provied, i already plan on doing it in some time)

src/typegoose.ts Outdated Show resolved Hide resolved
src/typegoose.ts Outdated Show resolved Hide resolved
src/typegoose.ts Outdated Show resolved Hide resolved
@naorzr
Copy link
Author

naorzr commented Jul 14, 2019

@hasezoey haven't created any special test for it as of now, might create some tests for it later on this week.

@hasezoey

This comment has been minimized.

@hasezoey
Copy link
Contributor

@naorzr could you then squash your fixup commits, thanks?

Update to that: could you resolve the merge conflicts?

@hasezoey hasezoey mentioned this pull request Jul 16, 2019
@naorzr
Copy link
Author

naorzr commented Jul 16, 2019

@hasezoey squashed and pushed.
these changes to package-lock.json files are straight up npm audit fix generated. later I saw a diff pr to fix these, do you want to wait for the pr? or just accept my changes?

@hasezoey
Copy link
Contributor

hasezoey commented Jul 16, 2019

@naorzr, sorry but it dosnt seem like you did it right, because the old commits are still there, you know you can rebase with squash and then force-push?

and the PR's that fix these audits are already merged

…its)

Squashed commits:
[6e6821f] audit fix
[580f4db] fix issues that caused tests to fail
[9c59b69] return schema and fix generic type
[7689d22] change buildPublicSchema to buildSchema, add jsDocs (+3 squashed commits)
Squashed commits:
[6e6821f] audit fix
[580f4db] fix issues that caused tests to fail
[9c59b69] return schema and fix generic type
@naorzr
Copy link
Author

naorzr commented Jul 16, 2019

@hasezoey should have forced push from begin with, thanks for the tip, it didn't work because i pulled after doing the squashing at the first time.. which resulted in back to square one.

As for the package-lock.json, I couldn't resolve these conflicts. it's best to just do an npm i && npm audit fix for the latest branch and use that package-lock.

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

Successfully merging this pull request may close these issues.

3 participants