@accounts/types : add a types package which contains common types #173

Aetherall · 2018-03-12T13:33:30Z

No description provided.

codecov-io · 2018-03-12T13:49:55Z

Codecov Report

Merging #173 into master will increase coverage by 0.08%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #173      +/-   ##
==========================================
+ Coverage   92.16%   92.24%   +0.08%     
==========================================
  Files          42       43       +1     
  Lines        1034     1045      +11     
  Branches      124      124              
==========================================
+ Hits          953      964      +11     
  Misses         70       70              
  Partials       11       11

Impacted Files	Coverage Δ
packages/types/src/index.ts	`100% <100%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e120cf5...edeab6a. Read the comment docs.

pradel · 2018-03-13T08:26:07Z

Why do we need a new types package, can this go to common instead?

Aetherall · 2018-03-13T08:33:00Z

In fact the common package actually is a dependency in some packages and a devDependency in others
the common package exports utilities that will no longer need to be shared across the suite in the future versions of Accounts-Js
(actually most of them are already moved to the corresponding package)

That's why I am creating @accounts/types and @accounts/error

@accounts/error will be the dependency
@accounts/types will be the devDependency

It keeps the relation between packages simple

davidyaha

These are only common types right?
Maybe add a readme that explains that

davidyaha · 2018-03-13T08:58:26Z

packages/types/package.json

+  },
+  "repository": {
+    "type": "git",
+    "url": "https://github.com/js-accounts/accounts/tree/master/packages/server"


https://github.com/js-accounts/accounts/tree/master/packages/types

Oh yes I am changing that

Aetherall · 2018-03-13T09:07:18Z

There is also DatabaseInterface because it makes more sense to have it here
I am adding the readme

davidyaha · 2018-03-13T09:26:40Z

@Aetherall A user with TS in his project needs to add this package to devDependencies as well?

Aetherall · 2018-03-13T09:33:39Z

@davidyaha only if he is using those types I suppose...

davidyaha · 2018-03-13T09:47:26Z

packages/types/readme.md

+
+It contains the types used across the Accouts Suite.
+
+#### Usage : This package is used internally by the suite. It's not needed to install it


OK, so maybe remove that.

pradel · 2018-03-13T10:35:16Z

packages/types/src/index.ts

+export { ImpersonationResult as ImpersonateReturnType } from './types/impersonation-result'
+
+
+// ? Operation Parameters


maybe we will need to figure this before merging no?

Yes actually I want to discuss it ASAP

So this will be used on the password server and password client right?

the @accounts/types package will be used on pretty much every @accounts package, as a devDependency

I mean the type

Yes it is used already

What is the idea of Operation Parameters?

For several operations, the function expect one object as parameters which contains all the informations the operation needs to execute

Actually this is not consistent because some operation are using this pattern and others are not

I would prefer to not use this way in order to simplify the workflow, but for this package to be compatible with the actual state suite, those types are needed

pradel · 2018-03-13T10:35:52Z

packages/types/src/types/user.ts

+
+export interface User {
+  username?: string;
+  email?: string;


is this line accurate?

In fact I just copied the types and renamed it
But if you look at the index.ts it's all renamed as original for compatibility
It's a part of the type renaming we will discuss in a meeting ( the sooner the better )

Yeah I now but was wondering if this is used or not ^^

I think it should not be used no?

you mean the email:string field?
or the whole User Interface ?

email string field

I agree, this was on the UserObjectType interface
I do not use it in the next branch neither
I was assuming the client used it
I am removing this right now

Aetherall added 5 commits March 12, 2018 13:26

@accounts/types : create package types

b5e0662

@accounts/types : added types from @accounts/common

799314d

@accounts/types : added DatabaseInterface

87f1d79

@accounts/types : added ConnectionInformations

310c510

Added tsconfig and test

8b15abf

Aetherall changed the title ~~@accounts/types add a types package which contains common types~~ @accounts/types : add a types package which contains common types Mar 12, 2018

Aetherall requested review from TimMikeladze, davidyaha and pradel March 12, 2018 15:39

davidyaha approved these changes Mar 13, 2018

View reviewed changes

Added reamde and fixed package.json

34f70da

davidyaha requested changes Mar 13, 2018

View reviewed changes

@accounts/types : readme.md Remove usage

67c204d

pradel reviewed Mar 13, 2018

View reviewed changes

@accounts/types : User interface - removed email?: string property

edeab6a

Aetherall mentioned this pull request Mar 13, 2018

AccountsJS Improvements #181

Open

12 tasks

davidyaha approved these changes Mar 15, 2018

View reviewed changes

davidyaha merged commit 34ec9db into master Mar 15, 2018

davidyaha deleted the package/types branch March 15, 2018 17:15


		It contains the types used across the Accouts Suite.

		#### Usage : This package is used internally by the suite. It's not needed to install it

		export { ImpersonationResult as ImpersonateReturnType } from './types/impersonation-result'


		// ? Operation Parameters

Uh oh!

@accounts/types : add a types package which contains common types #173

@accounts/types : add a types package which contains common types #173

Uh oh!

Conversation

Aetherall commented Mar 12, 2018

Uh oh!

codecov-io commented Mar 12, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

pradel commented Mar 13, 2018

Uh oh!

Aetherall commented Mar 13, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

davidyaha left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Aetherall commented Mar 13, 2018

Uh oh!

davidyaha commented Mar 13, 2018

Uh oh!

Aetherall commented Mar 13, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pradel Mar 13, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Aetherall Mar 13, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Aetherall Mar 13, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Aetherall Mar 14, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Aetherall Mar 13, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Aetherall Mar 13, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Aetherall Mar 13, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

codecov-io commented Mar 12, 2018 •

edited

Loading

Aetherall commented Mar 13, 2018 •

edited

Loading

pradel Mar 13, 2018 •

edited

Loading

Aetherall Mar 13, 2018 •

edited

Loading

Aetherall Mar 13, 2018 •

edited

Loading

Aetherall Mar 14, 2018 •

edited

Loading

Aetherall Mar 13, 2018 •

edited

Loading

Aetherall Mar 13, 2018 •

edited

Loading

Aetherall Mar 13, 2018 •

edited

Loading