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.

Sign up for GitHub

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

Jump to bottom

replaceAll-workaround #3348

Merged
merged 1 commit into from
Feb 22, 2023
Merged

replaceAll-workaround #3348

merged 1 commit into from
Feb 22, 2023

Conversation

haslinghuis
Copy link
Member

@haslinghuis haslinghuis commented Feb 19, 2023 •
edited by nerdCopter
Loading

@haslinghuis haslinghuis added this to the 10.10.0 milestone Feb 19, 2023
@haslinghuis haslinghuis self-assigned this Feb 19, 2023
@haslinghuis haslinghuis force-pushed the fix-replaceall branch 2 times, most recently from cb0bdb1 to 05fb3b9 Compare February 19, 2023 01:44
@github-actions

This comment has been minimized.

Sign in to view
@github-actions

This comment has been minimized.

Sign in to view
@github-actions

This comment has been minimized.

Sign in to view
@github-actions

This comment has been minimized.

Sign in to view
@blckmn
Copy link
Member

blckmn commented Feb 19, 2023 •
edited
Loading

AUTOMERGE: (FAIL)

  • github identifies PR as mergeable -> FAIL
  • assigned to a milestone -> PASS
  • cooling off period lapsed -> PASS
  • commit count less or equal to three -> PASS
  • Don't merge label NOT found -> PASS
  • at least one RN: label found -> PASS
  • Tested label found -> PASS
  • assigned to an approver -> PASS
  • approver count at least three -> FAIL

@nerdCopter
Copy link
Member

Should this be limited to Android only?
maybe something like

    if (/android/i.test(userAgent)) {
        [code]
    }

@nerdCopter
Copy link
Member

Didn't seem to break anything on Linux, but only lightly tested. Might prefer limiting to Android just in case as stated above.

@haslinghuis
Copy link
Member Author

haslinghuis commented Feb 19, 2023 •
edited
Loading

Yes tried yesterday on line 187 - adding now.

userAgent is not defined :)

@github-actions

This comment has been minimized.

Sign in to view
@nerdCopter
Copy link
Member

nerdCopter commented Feb 19, 2023 •
edited
Loading

Do you want to test this code? Here you have an automated build: Betaflight-Configurator-Android

I was able to use it on an Android 9 (Jan 2020) phone. Basic PIDs/Ports/CLI tabs. Did not test others.
EDIT: latest "nightly" worked as well, so not a fair test.

chmelevskij
chmelevskij reviewed Feb 22, 2023
View reviewed changes
src/js/main.js Outdated
Comment on lines 23 to 27
if (/android/i.test(navigator.userAgent)) {
if (typeof String.prototype.replaceAll === "undefined") {
String.prototype.replaceAll = function(match, replace) {
return this.replace(new RegExp(match, 'g'), () => replace);
};
}
}
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we need that android test tbh. typeof sort of covers all the required bases imo.

Copy link
Member

Choose a reason for hiding this comment

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

thank you; sounds fair. if that's the case @haslinghuis can revert and it can just be merged since it was tested the other way already,

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@github-actions
Copy link
Contributor

Do you want to test this code? Here you have an automated build:
Betaflight-Configurator-Android
Betaflight-Configurator-Linux
Betaflight-Configurator-macOS
Betaflight-Configurator-Windows
WARNING: It may be unstable and result in corrupted configurations or data loss. Use only for testing!

@haslinghuis haslinghuis merged commit 62f5cec into betaflight:master Feb 22, 2023
@haslinghuis haslinghuis deleted the fix-replaceall branch February 22, 2023 14:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@blckmn blckmn blckmn approved these changes

@chmelevskij chmelevskij chmelevskij approved these changes

@nerdCopter nerdCopter nerdCopter approved these changes

Assignees

@haslinghuis haslinghuis

Labels
RN: BUGFIX Tested
Projects
Finalizing Firmware 4.5 Release
Status: COMPLETED
Milestone
10.10.0
Development

Successfully merging this pull request may close these issues.
4 participants
@haslinghuis @blckmn @nerdCopter @chmelevskij