App: [HOLD for payment 2022-10-24] [$250] Resolve cyclic dependencies lint errors

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

There are some silenced cyclic dependency lint errors in our codebase. They were silenced in this PR as we didn’t want to block our project just because of them. Cyclic dependency doesn’t break anything but it’s good to optimize code and get rid of them.

Action Performed:

Break down in numbered steps

Uncomment // eslint-disable-next-line import/no-cycle lines in codebase
Run npm run lint

Expected Result:

No lint errors should show

Actual Result:

Cyclic dependency lint error appears

Workaround:

Can the user still use Expensify without this being fixed? Have you informed them of the workaround? Users are not affected

Platform:

Where is this issue occurring?

Web
iOS
Android
Desktop App
Mobile Web

Version Number: Reproducible in staging?: Reproducible in production?: Email or phone of affected tester (no customers): Logs: https://stackoverflow.com/c/expensify/questions/4856 Notes/Photos/Videos: Any additional supporting documentation Upwork job URL: https://www.upwork.com/jobs/~0105a5aec0a694b518 Issue reported by: Slack conversation:

View all open jobs on GitHub

About this issue

Original URL
State: closed
Created 2 years ago
Comments: 24 (19 by maintainers)

Most upvoted comments

Haha sorry if that was confusing @aimane-chnaif you haven’t done anything wrong here! 💙

This is totally semantics at this point, but let me break it down for clarity. And let me know if you think any external documentation has made this confusing for you as a contributor - I’ll gladly revise it, if so:

We perform a Root Cause Analysis (RCA) when there is a regression.
We use the term regression to say something used to work, but then broke and stopped working.
We do a RCA to identify the root cause of the regression itself and ultimately prevent it from happening again. So we don’t get repeated bugs in the same area of the code.
If it’s a new bug, then we don’t call it a regression. So new bugs don’t require an RCA.

michaelhaxhiu on Oct 9, 2022

Prefer to use Root cause for explaining the cause. And RCA or Regression when you want to refer to regression.

parasharrajat on Oct 9, 2022

Stepping in to help cover @stitesExpensify while he’s OOO

roryabraham on Oct 12, 2022

Though it’s a new bug and not a regression, I think it’s better to explain why this happens, rather than proposing only a solution.

Totally agree! As you continue to work on jobs, feel free to identify the “root cause” in your Problem statement like @parasharrajat mentioned 😃 love it.

michaelhaxhiu on Oct 9, 2022

@michaelhaxhiu Thanks for clarification. Though it’s a new bug and not a regression, I think it’s better to explain why this happens, rather than proposing only a solution. This can be labelled as Root Cause as @parasharrajat suggested?

aimane-chnaif on Oct 9, 2022

Just one question @aimane-chnaif - why are we saying RCA? Are we suggesting this is a regression (i.e. it used to work properly, but now it doesn’t)?

@michaelhaxhiu I just explained detail about those lint errors. you can ignore that if RCA means only a regression keyword

aimane-chnaif on Oct 9, 2022