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

Fix errors related to IDEA #25

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

hastebrot
Copy link
Contributor

This is my warmup PR. 😃

It adds IDEA project files to .gitignore and fixes some error messages in IDEA, so that IDEA instead shows unchecked assignment warnings.

@hastebrot
Copy link
Contributor Author

Note that I had three assertions fails unrelated to this change. Strangely they were gone after a second test run.

reactfx-assertions-fails

@hastebrot
Copy link
Contributor Author

The error message appeared in IDEA 14.1.4 and 142.4083.2. I first thought, this might be an EAP regression.

@TomasMikula
Copy link
Owner

Thanks for the PR. I'm not a big fan of unchecked assignments, because bugs can hide in them. Is there another way to make IDEA happy? For example, would the following make it happy:

Function<T, ObservableValue<U>> g = t -> f.apply(t);
return new FlatMappedVal<>(src, g);

Plus, to prevent someone else coming along this code and realizing the cast/extra variable is unnecessary and removing it, it should be documented that it is a workaround for a bug in IDEA compiler, with a link to their bug tracking system (so that the workaround can eventually be removed once the bug is resolved).

@TomasMikula
Copy link
Owner

Regarding the failing tests, it is a problem with the tests themselves. All those tests have to do with timing and break if scheduled events don't occur within an expected time window, typically if the system is under heavy load. I used to see those tests failing occasionally on my old laptop. The right thing to do would be to make the tests robust.

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.

2 participants