Skip to content
This repository has been archived by the owner on Oct 26, 2021. It is now read-only.

Symbol field and metadata handling in 5.1 #446

Closed

Conversation

poeschlr
Copy link
Contributor

@poeschlr poeschlr commented Oct 6, 2019

Version 5.1 introduced a new GUI that no longer closely resembles how
this information is stored in the files. This means the rules 6.2 and
6.3 no longer make sense to be separated. It also required updating of
screenshots.

Script updates found in KiCad/kicad-library-utils#312
fixes #442

Version 5.1 introduced a new GUI that no longer closely resembles how
this information is stored in the files. This means the rules 6.2 and
6.3 no longer make sense to be separated. It also required updating of
screenshots.
@poeschlr poeschlr force-pushed the update_datasheet_handling_to_v5.1 branch from dc9e7a7 to 67fd70d Compare October 6, 2019 15:36
. *Footprint* field contains footprint link for fully specified symbols, and is empty for generic symbols. It must be _invisible_
. *Datasheet* entry [1] is filled out, and is _invisible_
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This style of using a numbered reference in brackets is not used elsewhere in KLC that I can find. I see a a separate section below without any linking or sub-bullets. Do you want to introduce this new style?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a footnote for adding additional information. We do not have a footnote anywhere else in the KLC (it was not necessary to clarify things like that in other rules. As this is really one of the few places where we need to explain strange KiCad behaviour)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not clear to me why this needs to be done this way instead of just being underneath the Datasheet section. But OK.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wanted to keep this rule short so that users read it. What is below basically gives additional clarification for users who are still interested.

I learned with bad experiences that users turn their brain off after a few sentences so the most important info should be kept short. Adding additional stuff for the few users that are interested is still worth it. Especially if we ever get a discussion in a pull request because then we can point to this additional information.

. *Footprint* field contains footprint link for fully specified symbols, and is empty for generic symbols. It must be _invisible_
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other bullets here would replace the . It must be with and is.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed this section to be more readable

@poeschlr
Copy link
Contributor Author

I fixed the merge conflict and fixed what needed fixing

@evanshultz
Copy link
Contributor

Just two things:

  1. I believe you need a colon after via.
  2. None of the clauses end in a period. I checked S6.1 and those clauses end in periods. But S7.1 doesn't, and earlier symbol clauses are mixed. I assume you'd rather merge this and then do cleanup to all of KLC at once. But I'm noting it here so you're aware.

@poeschlr
Copy link
Contributor Author

Harmonizing the use of periods at the end of KLC list entries would definitely be a separate pull request.

I am not sure what you mean with your other request.

. The symbol contains no other custom fields

Additional documentation is provided via
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a colon at the end of this line.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see a comma, not a colon. I was shooting for path of least resistance before, but since this isn't closed I feel the wording is a bit awkward and would read better if two other fields: was added after via.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remember you might need to be a bit clearer with what you mean when writing with non native speakers. For example you could have added the symbol you were expecting to your comment (as the symbol itself in addition to its english name)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right. I will try to be more clear in the future. There are a few things I could have done better.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I imagine it is hard to remember that not everyone here is a native speaker as our knowledge of english is well enough to hide this fact most of the time.

---

_[1]: KiCad 5.x and earlier have two places to store the datasheet link. It can be in the `.lib` file once per symbol or in the `.dcm` file allowing different datasheets per alias. Having different datasheets for aliases only works if the `.lib` entry is left empty. The datasheet entry in the main symbol metadata view is linkded to the dcm entry since version 5.1.x._
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo of linked in the last sentence. Remove the first d in linkded.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

@evanshultz
Copy link
Contributor

Great. Thank you!

@evanshultz
Copy link
Contributor

@nickoe @marekr
This PR is ready to be merged, assuming it's still OK to merge PRs from GH. Let us know if you are only merging MRs from Gl for the website.

@nickoe
Copy link
Contributor

nickoe commented Jan 18, 2020

@evanshultz Ok, thanks for the mention, but it looks like there is a conflict so I can't make github rebase it automatically. I will try to remember to fix it tomorrow unless someone beats me to it. @marekr

@poeschlr
Copy link
Contributor Author

poeschlr commented Jan 18, 2020

Github does not show a conflict to me.

Edit: If the website already moved to gitlab then we might need to run the bot to get the updates over. (I continued to work here because i seem to remember i read somewhere that the website is still on github)

@nickoe
Copy link
Contributor

nickoe commented Jan 18, 2020

We still use github for the website for now.
I resolved the conflict and squashed the fixes into 9c9cb33. Thank you.

@nickoe nickoe closed this Jan 18, 2020
@poeschlr
Copy link
Contributor Author

@evanshultz could you in this case look at the script pull request? The script would otherwise point to the wrong KLC rule(s).

@poeschlr poeschlr deleted the update_datasheet_handling_to_v5.1 branch January 18, 2020 12:08
@poeschlr
Copy link
Contributor Author

@nickoe are you sure you merged the correct commit range? The klc history page does not show the result of my merge.

@nickoe
Copy link
Contributor

nickoe commented Jan 18, 2020

I see 3.0.25 in the changeset 9c9cb33#diff-87d0795c9f659b7ebd57e1b84e1c2435

@poeschlr
Copy link
Contributor Author

The date should be 3.0.25 - 2020-01-13

@nickoe
Copy link
Contributor

nickoe commented Jan 18, 2020

Yes, the date is already fixed.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

KLC S6.2 & S6.3: How to handle datasheet?
3 participants