Skip to content

Conversation

@mbharat
Copy link

@mbharat mbharat commented Feb 10, 2018

This patch appends "init" to the labels of the initial conditions for the capacitor
and inductor. Also fixed a typo in capacitor class implementation.

This patch appends "init" to the labels of the initial conditions for the capacitor
and inductor.
@felix-salfelder
Copy link
Member

Vinit seems wrong. see e.g. qucs-core/src/components/capacitor.cpp line 142.

there's a real problem to it: Props must be private. (with that fixed, you could easily introduce aliases.)

@in3otd
Copy link
Contributor

in3otd commented Feb 10, 2018

um, I think this will break backward compatibility, as it changes the qucsator netlist format. If one uses (library) subcircuits with capacitors or inductors they will not work anymore.
Sure, we could handle this as a special case but honestly I do not see a good reason to do this change at all; what V and I stands for is explained in the Propery description, shown in the Edit Property dialog box. If further explanations are needed, those should not go into the Property name but into a proper Help document, the Qucs Reference Manual (and, yes, maybe one day I'll finish #448, so that help can easily be accessed from the component Edit Property dialog).

@mbharat
Copy link
Author

mbharat commented Feb 10, 2018

Thanks for the clarifications all.

I suggest to withdraw this pull request. I think the best way to contribute to QUCS development is to look through the various issues and find one that interests me.

How do I go about "abandoning" this pull request?

Thx.

Bharath

@in3otd
Copy link
Contributor

in3otd commented Feb 10, 2018

No problem Bharath, it's a good thing you previously sent your proposals to the qucs-devel mailing list, unfortunately we were slow to follow up so we couldn't start a discussion there. I'll try to comment further there later.

You should see a button Close and comment under here, this will mark it as closed and make it disappear from the list of things to review/merge.

@mbharat
Copy link
Author

mbharat commented Feb 10, 2018

Closing (abandoning) this change as the approach is incorrect and overall patch is unnecessary.

@mbharat mbharat closed this Feb 10, 2018
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