-
Notifications
You must be signed in to change notification settings - Fork 72
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
Add devDeps for flow-bin
and postcss
#1479
Conversation
@sjd78 |
not exactly connected with
|
... #1485 |
There are some breaking changes in flow from the version used to current. Plus some of the library types referenced in flow could/will also break with upgrade without being upgraded at the same time. ImmutableJS is like that. I don't want to touch those quite yet, just turn flow back on until there is some time for #1452. |
- Add `flow-bin` to enable manual flow checking. The version added is the last version used by the project. To run flow, use the command: yarn flow check - Explicitly add `postcss` to remove the `yarn install` warning about missing peer dependencies. This version was already referenced in yarn.lock so no new dependencies have been added.
dba9ea7
to
2dc3779
Compare
ci test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, except that after I run yarn flow check
, I get the error:
$ /home/hstastna/oVirt_/ovirt-web-ui/node_modules/.bin/flow check
Error ┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈ node_modules/immutable/dist/immutable.js.flow:406:33
SetSeq [1] is incompatible with IndexedSeq [2].
[2] 387│ static of<T>(...values: T[]): IndexedSeq<T>;
:
403│
404│ declare class SetSeq<T> extends Seq<T,T> mixins SetIterable<T> {
405│ static <T>(iter?: ESIterable<T>): IndexedSeq<T>;
[1] 406│ static of<T>(...values: T[]): SetSeq<T>;
407│ }
408│
409│ declare class List<T> extends IndexedCollection<T> {
This seems related to a bug in flow definitions for Immutable. |
This is an old error that we ignored before. I'm fine with keeping it since flow type checks are purely manual/ide based right now. From my answer to Radek's similar question before, seems risky to upgrade flow or immutable. I'd rather spend time to swap to typescript than fix flow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Add pre-seed for changes: - https://gerrit.ovirt.org/c/ovirt-engine-ui-extensions/+/115519 - https://gerrit.ovirt.org/c/ovirt-engine-ui-extensions/+/115816 - oVirt/ovirt-web-ui#1479 Remove for merged changes: - oVirt/ovirt-web-ui#1460 Change-Id: I5592dcc6f3f1619caf7a383d10cfb19a037ea332 Signed-off-by: Scott J Dickerson <[email protected]>
Add pre-seed for changes: - oVirt/ovirt-web-ui#1497 Remove pre-seeds for merged/closed changes: - oVirt/ovirt-web-ui#1490 - oVirt/ovirt-web-ui#1479 - https://gerrit.ovirt.org/c/ovirt-engine-ui-extensions/+/115819 - https://gerrit.ovirt.org/c/ovirt-engine-ui-extensions/+/115519 - https://gerrit.ovirt.org/c/ovirt-engine-ui-extensions/+/115816 Change-Id: I4f19018533c8280fefea976c0b7884f4417ea615 Signed-off-by: Sharon Gratch <[email protected]>
flow-bin
to enable manual flow checking. The version added is the last version used by the project. To run flow, use the command:postcss
to remove theyarn install
warning about missing peer dependencies. This version was already referenced in yarn.lock so no new dependencies have been added. The error fixed is this one on a yarn install: