joeyaurel / python-gedcom

Python module for parsing, analyzing, and manipulating GEDCOM files
https://gedcom.joeyaurel.dev
GNU General Public License v2.0
154 stars 39 forks source link

Add missing 5.5.1 tags and some of the common program defined ones. #49

Open cdhorn opened 4 years ago

cdhorn commented 4 years ago

Small patch to add all the missing tags from the 5.5.1 standard as well as some of the more common program defined ones.

ghost commented 4 years ago

There were the following issues with this Pull Request

You may need to change the commit messages to comply with the repository contributing guidelines.


🤖 This comment was generated by commitlint[bot]. Please report issues here.

Happy coding!

cdhorn commented 4 years ago

I'm guessing I goofed, first time trying this. If you see what I did wrong please let me know. Thanks.

joeyaurel commented 4 years ago

Thank you for your contribution! Left some change requests behind :)

I haven't documented it well yet (because it's not in the master branch and not in the main README.md) but here in the contribution guidelines you see how to commit changes using conventional commits. This is why "commitlint" found problems.

Travis CI failed because there are files missing for the MANIFEST check. If you could do me the favor and adding the following to your MANIFEST.in:

recursive-exclude tests *

And removing:

include tests/files/*.ged

Would much appreciate it!

ghost commented 4 years ago

There were the following issues with this Pull Request

You may need to change the commit messages to comply with the repository contributing guidelines.


🤖 This comment was generated by commitlint[bot]. Please report issues here.

Happy coding!

cdhorn commented 4 years ago

Updated the manifest and folded all your recommendations in as good ones and consistent. Those are two new commits on my side following the conventional commit guidelines. Not sure if that is best way to do that, first time contributing to someone else's project. Is that a new pull request or this one still works? Thanks!

cdhorn commented 4 years ago

And now I'm confused. I used commitizen and ran them through commitlint via a git hook. Did I miss something?

joeyaurel commented 4 years ago

@cdhorn You did everything right this time! But your first two commits f14c4009ddb3879e69381c36196ee2760f9a71ca and 4cf4cba507be2d05d1a0a5f64045c0ce60a24631 have the wrong message format.

As the cute little commitlint bot said, you need to change these two commit messages.

cdhorn commented 4 years ago

Ah, okay, thank you. Have spent this morning reading the 5.5.5 standard, need to digest it. The module probably needs to be refactored with a separate reader for 5.5, 5.5.1 and 5.5.5. The 5.5.5 standard clarifies lots of ambiguities in prior versions, eliminates dead baggage all over, and requires strict adherence unlike prior versions. Good info on best practice for reading/writing the three different types in there, how to differentiate between 5.5 and 5.5.1 and more. Have you read the updated standard yet? Are you planning any significant architectural changes for the module in the near future?

cdhorn commented 4 years ago

More research, I see some controversy over the new standard as it was not from FamilySearch.