search

hildogjr / KiCost

Build cost spreadsheet for a KiCad project.
MIT License
519 stars 98 forks source link

Testcase for #266 #483

Closed mdeweerd closed 3 years ago

mdeweerd commented 3 years ago

This adds a testcase for #266 which makes it easer to check it.

The manf# and manf field should propagate for SubParts too. The results of the testcase are still not as expected.

set-soft commented 3 years ago

Hi @mdeweerd !

Thanks for it, but I don't think merging it is a good idea. Not until the solution is implemented, or the team chooses not to implement it. Otherwise people not familiar with this issue could think that propagating is wrong, when the user is asking for it ;-)

Perhaps @hildogjr wants to merge it. I copied it to a branch called mdeweerd-266 in case @hildogjr wants to play with it.

BTW: please use "flake8" to check your patches.

mdeweerd commented 3 years ago

@set-soft Thank you for the feedback.
I think it is easier to have the test case in the master - leaving it in a branch just lets it go stale. I referenced the issue index.
In the master it is also easier to check what the result of the test case is. Otherwise there is an extra effort to check the branch.
Il try to think about checking with flake8 - maybe it should be added to the test automation so that it automatically stands out af "failing" for styling errors.

set-soft commented 3 years ago

Ok, lets @hildogjr take a decision.

I think regression tests must reflect what we want to keep as behavior, not just issues.

We could add some special file and or directory for the purpose you mention, but mixing stablished behavior with random stuff isn't a good practice.

set-soft commented 3 years ago

I manually merged it, but:

  1. I disabled the test. Is there but not executed.
  2. I simplified the enable/disable for ADD_QUERY_TO_KNOWN
set-soft commented 3 years ago

BTW: I use "Undo" to revert it

mdeweerd commented 3 years ago

I mostly use vi, so with the previous method I just do "x:w" or "xx:w" and then "i# :w" at the start of the line, or "U:w" if it was the last change ;-). With new method "$r0:w" and then "$r1:w" or "U:w" . The "comment/uncomment" method works in most languages, I tend to "unify" techniques. Some editors it's just "<CTRL+SHIFT+/><CTRL+s>" for both operations.