Skip to content

Conversation

@Cabalist
Copy link

Hey there,

Just started using this. I noticed that files writes were not using a contextmanager and then fell down the rabbit hole of cleaning up some other things. Nothing should be a functional change and the with open(...) as ... will prevent any dangling filehandlers should an issue happen.

Each commit should be self contained to the issue it addresses for review instead of seeing a huge amount of thrash.

Ignoring whitespace in the compare is also helpful to cut down on the noise. :)

Cheers! Thanks for an awesome project!

@JamesH65
Copy link
Contributor

Can you post the functional changes as a separate PR please. Also, I disagree with some of the whitespace changes, there was formatting in there for a reason. Hence separating from the functional changes.

@Cabalist
Copy link
Author

Done. New PR opened (#33)

As for the purposeful whitespace formatting, I see

features_list = {
'spi' : ("SPI", "spi.c", "hardware/spi.h", "hardware_spi"),
'i2c' : ("I2C interface", "i2c.c", "hardware/i2c.h", "hardware_i2c"),
'dma' : ("DMA support", "dma.c", "hardware/dma.h", "hardware_dma"),
'pio' : ("PIO interface", "pio.c", "hardware/pio.h", "hardware_pio"),
'interp' : ("HW interpolation", "interp.c", "hardware/interp.h", "hardware_interp"),
'timer' : ("HW timer", "timer.c", "hardware/timer.h", "hardware_timer"),
'watch' : ("HW watchdog", "watch.c", "hardware/watchdog.h", "hardware_watchdog"),
'clocks' : ("HW clocks", "clocks.c", "hardware/clocks.h", "hardware_clocks"),
}
stdlib_examples_list = {
'uart': ("UART", "uart.c", "hardware/uart.h", "hardware_uart"),
'gpio' : ("GPIO interface", "gpio.c", "hardware/gpio.h", "hardware_gpio"),
'div' : ("Low level HW Divider", "divider.c", "hardware/divider.h", "hardware_divider")
}
Are there any other places were whitespace has purpose/meaning that were overwritten?

@JamesH65
Copy link
Contributor

I'll have to check again, that was the obvious example.

@lurch
Copy link
Contributor

lurch commented Sep 2, 2021

Has this been superseded by #39 ?

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

Successfully merging this pull request may close these issues.

3 participants