Skip to content

[CHORE] Only run Flipper in debug via MainApplication is debug#2347

Merged
diegolmello merged 2 commits intoRocketChat:developfrom
grahamsmith:remove-flipper-reflection
Jul 30, 2020
Merged

[CHORE] Only run Flipper in debug via MainApplication is debug#2347
diegolmello merged 2 commits intoRocketChat:developfrom
grahamsmith:remove-flipper-reflection

Conversation

@grahamsmith
Copy link
Copy Markdown
Contributor

@grahamsmith grahamsmith commented Jul 28, 2020

Proposed changes

These changes remove the need for reflection to load Flipper. They also fix the need to hard code the package name, which is not helpful when white labelling the RC App.

Issue(s)

How to test or reproduce

No UI/UX changes and no dev flow changes. Flipper should work how it did previously.

Screenshots

Types of changes

  • [x ] Bugfix (non-breaking change which fixes an issue)
  • [x ] Improvement (non-breaking change which improves a current function)
  • New feature (non-breaking change which adds functionality)
  • Documentation update (if none of the other choices apply)

Checklist

  • [ x] I have read the CONTRIBUTING doc
  • [ x] I have signed the CLA
  • [ x] Lint and unit tests pass locally with my changes
  • [x ] I have added tests that prove my fix is effective or that my feature works (if applicable)
  • [x ] I have added necessary documentation (if applicable)
  • [x ] Any dependent changes have been merged and published in downstream modules

Further comments

@Fishbowler
Copy link
Copy Markdown
Contributor

Fishbowler commented Jul 28, 2020

LGTM

@grahamsmith grahamsmith force-pushed the remove-flipper-reflection branch from 7aaea4c to 4281bc9 Compare July 30, 2020 07:17
@grahamsmith
Copy link
Copy Markdown
Contributor Author

grahamsmith commented Jul 30, 2020

Screenshot 2020-07-30 at 08 45 59

@grahamsmith
Copy link
Copy Markdown
Contributor Author

grahamsmith commented Jul 30, 2020

@djorkaeffalexandre @diegolmello - I have double checked everything and ran the Android App with Flipper running (see screenshot). All is working 👍

The confusion was around someone previously copying and pasting some code and not changing the package name. I fixed this in the latest commit.

I had to bump Gradle to 6.3 as I run JDK 14 and Gradle will not build on versions before this.

In another PR I shall bring Flipper up to date as its a few versions behind.

@grahamsmith grahamsmith mentioned this pull request Jul 30, 2020
Merged
10 tasks
@diegolmello diegolmello changed the title Only run Flipper in debug via MainApplication is debug [CHORE] Only run Flipper in debug via MainApplication is debug Jul 30, 2020
diegolmello
diegolmello approved these changes Jul 30, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@diegolmello diegolmello left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@diegolmello diegolmello merged commit 72f7471 into RocketChat:develop Jul 30, 2020
diegolmello added a commit that referenced this pull request Sep 1, 2020
@diegolmello @djorkaeffalexandre
@youssef-md @Prateek93a @phriedrich @sarthakpranesh @devyaniChoubey @EzequielDeOliveira @regalstreak @dependabot @GOVINDDIXIT @azhaubassar @zhaubassar @Badal1234 @BrunoOliveiraDantas @CalebeRios @Fishbowler @snyk-bot @refactor-droidyy @grahamsmith @gabriellsh @juliagrala
Merge beta into master (#2445