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

Yellow boxes make developers anxious and potentially less productive #4014

Closed
jsamr opened this issue Jul 13, 2021 · 4 comments
Closed

Yellow boxes make developers anxious and potentially less productive #4014

jsamr opened this issue Jul 13, 2021 · 4 comments
Assignees

Comments

@jsamr
Copy link
Collaborator

jsamr commented Jul 13, 2021

Although I have to admit I don't have scientific evidence to back up this generalizing assertion, I have found that those yellow warnings:

  • distract me from my current task;
  • give the impression that "something is broken" thus raising my anxiety level.

Action Performed:

Launch the app on a mobile device in Dev mode.

Expected Result:

No anxiety.

Actual Result:

An anxiety spike.

image

Workaround:

LogBox.ignoreLogs(['Setting a timer for a long period of time', 'Require cycle: node_modules/rn-fetch-blob', 'If you want to use Reanimated 2']);

Discussion

Logs are important and require attention, so ignoring all logs would certainly be a bad idea. But letting these pop is bad for DX. Here is what I propose to handle each warning:

  1. "Setting a timer..." → basically it means that if the app goes in the background and back to foreground on Android, the timer is lost. Currently Expensify is using a 30 minutes interval to refresh personal details. See this great comment on this issue. Suggestion : track this issue in a separate ticket and ignore the log in the meantime.
  2. "Require cycle..." has no functional consequence, and is caused by rn-fetch-blob. A fix would require to upgrade to a maintained fork such as react-native-blob-util. Suggestion: ignore the log until this lib is dropped; optionally open a ticket to migrate.
  3. "If you want to use Reanimated 2 ..." → not an obvious answer, but could be related to react-navigation, see discussion. Suggestion: ignore the log, and create a low-priority issue to track it down.

As a general rule, I suggest when a new warning raises, one should:

  • Add the warning to the ignoreLogs array with the most specific string;
  • Find out if the warning should be permanently ignored or an issue should be open.

Platform:

iOS
Android


I'll be happy to address this as a gift to Expensify during my upwork job.

@jsamr jsamr added the AutoAssignerTriage Auto assign issues for triage to an available triage team member label Jul 13, 2021
@MelvinBot
Copy link

Triggered auto assignment to @lschurr (AutoAssignerTriage), see https://stackoverflow.com/c/expensify/questions/4749 for more details.

@MelvinBot MelvinBot removed the AutoAssignerTriage Auto assign issues for triage to an available triage team member label Jul 13, 2021
@marcaaron
Copy link
Contributor

Actual Result: An anxiety spike.

😂 This is great. Honestly, I think we have put off on ignoring them since we were maybe hoping to fix them. But I'm not against ignoring these particular warnings if we are not going to do anything about them.

@parasharrajat
Copy link
Member

"If you want to use Reanimated 2 ..." → not an obvious answer, but could be related to react-navigation, see discussion. Suggestion: ignore the log, and create a low-priority issue to track it down.

I am looking into upgrading to Animated 2 #3972

@jsamr
Copy link
Collaborator Author

jsamr commented Jul 14, 2021

@parasharrajat Thanks for the info. I'm going to prepare a PR for those boxes, and I'll remove the reanimated filter as this should be decided in #3972.

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

No branches or pull requests

5 participants