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

Update en.rst by removing reference to gramps-xml (Issue ID 0009723) #1630

Closed
wants to merge 1 commit into from

Conversation

kenhara
Copy link
Contributor

@kenhara kenhara commented Jan 16, 2024

I saw this old ticket and thought I might be able to clean it up: https://gramps-project.org/bugs/view.php?id=9723
This is my first contribution, sorry if I'm doing this incorrectly!

I saw this old ticket and thought I might be able to clean it up: https://gramps-project.org/bugs/view.php?id=9723
 This is my first contribution, sorry if I'm doing this incorrectly!
Copy link
Member

@hgohel hgohel left a comment

Choose a reason for hiding this comment

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

@kenhara Thanks for your first PR to Gramps!

A quick search of the code for gramps-xml shows that two similar command lines are part of the usage string in gramps\gramps\cli\argparser.py and should be updated with this PR.

@@ -221,7 +221,7 @@ To explicitly specify the formats in the above example, append filenames with
appropriate **-f** options::

gramps -i file1.ged -f gedcom -i file2.tgz -f gramps-pkg \
-i ~/db3.gramps -f gramps-xml -i file4.wft -f wft -a check
-i ~/db3.gramps -f gramps -i file4.wft -f wft -a check
Copy link
Member

Choose a reason for hiding this comment

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

As an argument, changing to -f gramps is correct. All the other instances of gramps-xml on this page refer to the file format which is "Gramps XML" format so I believe it is OK to leave them as-is. However I would defer to someone who has some historical knowledge of the change. @Nick-Hall or @emyoulation or...?

@hgohel
Copy link
Member

hgohel commented Jan 18, 2024

@kenhara BTW, I noticed this PR is against your master branch. Typically you'd want to create a branch in your fork for each PR you submit to Gramps. It can take a while for PRs to get merged and you don't want to have multiple fixes pending in your master branch - it will get difficult to manage.

@kenhara
Copy link
Contributor Author

kenhara commented Jan 18, 2024

@hgohel Thanks so much for your help, apologies for the newbie error. I've submitted a fresh PR that incorporates this feedback. It is from a branch (not master), it just focuses on the command line changes and adds the additional one you found. Feel free to merge the two or simply close this one, I hope creating another was alright:

([https://github.com//pull/1631])

@kenhara kenhara closed this Jan 18, 2024
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