search

SiLab-Bonn / basil

A data acquisition framework in Python and Verilog.
BSD 3-Clause "New" or "Revised" License
40 stars 29 forks source link

Flowmeter #220

Open YannickDieter opened 5 months ago

YannickDieter commented 5 months ago

Can you please comment a bit on the changes? To me it looks like a lot is simplyfied (most of the code is removed), but it still works?

YannickDieter commented 5 months ago

ping @matthias-schuessler

matthias-schuessler commented 5 months ago

I can't really comment on this, because this branch is entirely Antonios and Thomas work. I never worked with this branch and our current module qc environment does not use the changes in it. I can look into this.

YannickDieter commented 5 months ago

Yes, please have a look. In the longterm we want to use the flowmeter for our setup and it would be nice to check if the code changes actually work. Thanks! :)

cbespin commented 2 months ago

@matthias-schuessler Why do you always force-push? If it does not work with a regular push, your local history is messed up.

matthias-schuessler commented 2 months ago

@cbespin I did this, since I just performed a rebase of flowmeter onto the current version of master (basil release 3.3.0). From what I gathered from documentation, a force-push is necessary after a rebase since the idea behind rebase is to change the branch history.

matthias-schuessler commented 2 months ago

Yes, please have a look. In the longterm we want to use the flowmeter for our setup and it would be nice to check if the code changes actually work. Thanks! :)

The flowmeter changes in here are working properly after rebase and can be considered ready for merge.

cbespin commented 2 months ago

You do not need a force-push after a rebase. This is the idea of a rebase ;) force-push overwrites the remote history. Always try with a normal push and if you encounter errors, deal with them first before considering a force-push.

I added myself as reviewer and will take a look as soon as possible