Skip to content

Conversation

@haslinghuis
Copy link
Member

@haslinghuis haslinghuis commented Aug 18, 2021

Rebase of stm32.js:

  • fix lexical scoping
  • change strings into template literals
  • remove type coercion
  • change (most) variables from kebab_case to camelCase

Only one file to go (usbstm32dfu.js)

@haslinghuis haslinghuis added this to the 10.8.0 milestone Aug 18, 2021
@haslinghuis haslinghuis self-assigned this Aug 18, 2021
@haslinghuis haslinghuis force-pushed the fix_lexical_scoping branch 2 times, most recently from 7d62218 to 92d4840 Compare August 19, 2021 00:28
asizon
asizon previously approved these changes Aug 22, 2021
McGiverGim
McGiverGim previously approved these changes Aug 23, 2021
Copy link
Member

@McGiverGim McGiverGim left a comment

Choose a reason for hiding this comment

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

Good cleaning! I have not tested it, but seems good to me...

ctzsnooze
ctzsnooze previously approved these changes Aug 31, 2021
// send over the actual data
serial.send(bufferOut, function (writeInfo) {});
serial.send(bufferOut, function (writeInfo) {
//
Copy link
Member

Choose a reason for hiding this comment

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

Looking at src/js/serial.js, callback seems to be optional, so we can probably just omit this.

Copy link
Member Author

Choose a reason for hiding this comment

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

Reduced.

// result = true/false
STM32_protocol.prototype.verify_flash = function (first_array, second_array) {
for (var i = 0; i < first_array.length; i++) {
for (let i = 0; i < first_array.length; i++) {
Copy link
Member

Choose a reason for hiding this comment

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

Did you want to fix first_array as well?

Copy link
Member Author

Choose a reason for hiding this comment

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

Renamed

if (this.receive_buffer.length >= n_bytes) {
// data that we need are there, process immediately
var data = this.receive_buffer.slice(0, n_bytes);
const data = this.receive_buffer.slice(0, n_bytes);
Copy link
Member

Choose a reason for hiding this comment

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

Did you want to fix n_bytes as well?

Copy link
Member Author

Choose a reason for hiding this comment

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

Renamed

// write start address + checksum
self.send([addressArray[0], addressArray[1], addressArray[2], addressArray[3], addressChecksum], 1, function (_reply) {
if (self.verify_response(self.status.ACK, _reply)) {
const ArrayOut = Array.from(bytesToWrite + 2); // 2 byte overhead [N, ...., checksum]
Copy link
Member

Choose a reason for hiding this comment

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

Not sure why this is capitalised?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, as original, fixed now.

@haslinghuis haslinghuis dismissed stale reviews from ctzsnooze, McGiverGim, and asizon via 11c098e August 31, 2021 04:44
@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 3 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@haslinghuis
Copy link
Member Author

Do I need to do anything here to get it approved?

@blckmn
Copy link
Member

blckmn commented Sep 26, 2021

AUTOMERGE: (PASS)

  • github identifies PR as mergeable -> PASS
  • 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 -> PASS

@blckmn blckmn merged commit 1ffa40e into betaflight:master Sep 26, 2021
@haslinghuis haslinghuis deleted the fix_lexical_scoping branch September 26, 2021 03:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants