Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

nicify some docstrings and descriptions #232

Merged
merged 1 commit into from
Oct 18, 2023
Merged

nicify some docstrings and descriptions #232

merged 1 commit into from
Oct 18, 2023

Conversation

baggepinnen
Copy link
Contributor

After playing around with the GUI a bit, I realized that some descriptions were overly verbose and at the same time not as informative as they could be.

@baggepinnen baggepinnen requested a review from ven-k October 17, 2023 13:48
@ven-k
Copy link
Member

ven-k commented Oct 17, 2023

There were some discussions on whether we should add label metadata and use that for one word description like Resistance, Gain etc, while description can be long form description.

@codecov
Copy link

codecov bot commented Oct 17, 2023

Codecov Report

Merging #232 (eed1579) into main (1c7eb69) will not change coverage.
The diff coverage is 0.00%.

@@           Coverage Diff           @@
##             main     #232   +/-   ##
=======================================
  Coverage   55.34%   55.34%           
=======================================
  Files          48       48           
  Lines        1610     1610           
=======================================
  Hits          891      891           
  Misses        719      719           
Files Coverage Δ
src/Blocks/continuous.jl 90.97% <0.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@baggepinnen
Copy link
Contributor Author

Yeah, that could be useful. The descriptions that were shortened were so because they didn't actually provide anything meaningful with all their words: "Time constant of FirstOrder filter" is no more informative than "Time constant" since it's obvious from context, both in the GUI and in the REPL, that it belongs to the FirstORder filter

@baggepinnen baggepinnen merged commit 0f0fae1 into main Oct 18, 2023
6 of 9 checks passed
@baggepinnen baggepinnen deleted the fb/docstrings branch October 18, 2023 04:42
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.

2 participants