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

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

Open
wants to merge 14 commits into
base: develop
Choose a base branch
from

Conversation

cdhorn
Copy link

@cdhorn cdhorn commented May 15, 2020

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
Copy link

ghost commented May 15, 2020

There were the following issues with this Pull Request

  • Commit: 4cf4cba
    • ✖ message may not be empty
    • ✖ type may not be empty

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
Copy link
Author

cdhorn commented May 15, 2020

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

@joeyaurel joeyaurel added this to To do in Python GEDCOM Parser via automation May 15, 2020
@joeyaurel joeyaurel added this to the 2.0.0 milestone May 15, 2020
@joeyaurel joeyaurel self-assigned this May 15, 2020
gedcom/tags.py Outdated Show resolved Hide resolved
gedcom/tags.py Outdated Show resolved Hide resolved
gedcom/tags.py Outdated Show resolved Hide resolved
gedcom/tags.py Outdated Show resolved Hide resolved
gedcom/tags.py Outdated Show resolved Hide resolved
gedcom/tags.py Outdated Show resolved Hide resolved
gedcom/tags.py Outdated Show resolved Hide resolved
gedcom/tags.py Outdated Show resolved Hide resolved
gedcom/tags.py Outdated Show resolved Hide resolved
Python GEDCOM Parser automation moved this from To do to Review May 15, 2020
@joeyaurel joeyaurel changed the base branch from master to develop May 15, 2020 14:37
@joeyaurel
Copy link
Owner

joeyaurel commented May 15, 2020

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
Copy link

ghost commented May 16, 2020

There were the following issues with this Pull Request

  • Commit: f14c400
    • ✖ message may not be empty
    • ✖ type may not be empty
  • Commit: 4cf4cba
    • ✖ message may not be empty
    • ✖ type may not be empty

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
Copy link
Author

cdhorn commented May 16, 2020

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
Copy link
Author

cdhorn commented May 16, 2020

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

Python GEDCOM Parser automation moved this from Review to In progress May 16, 2020
@joeyaurel
Copy link
Owner

joeyaurel commented May 16, 2020

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

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

Repository owner deleted a comment May 16, 2020
Repository owner deleted a comment May 16, 2020
@cdhorn
Copy link
Author

cdhorn commented May 16, 2020

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
Copy link
Author

cdhorn commented May 16, 2020

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Python GEDCOM Parser
  
In progress
Development

Successfully merging this pull request may close these issues.

None yet

2 participants