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

Use parselglossy to generate parser and documentation #84

Merged
merged 18 commits into from
Jul 6, 2020

Conversation

robertodr
Copy link
Contributor

As discussed at length since the first few releases, I'd like parselglossy to become a generator for parser and documentation.
High-level usage:

parselglossy generate template.yml

output:

parser/
├── __init__.py
├── grammar.py
├── parser.py
├── README.md
├── cli.py
├── documentation.rst
└── pyparsing.py

Main goal is to have zero dependencies for final users. The CLI is generated with Argparse, while pyparsing is shipped with the generated files. This protects template.yml and the grammar from accidental tampering.

@codecov
Copy link

codecov bot commented Dec 24, 2019

Codecov Report

Merging #84 into master will decrease coverage by 1.08%.
The diff coverage is 90.69%.

@bast
Copy link
Member

bast commented Jan 7, 2020

I understand this is a draft pull request?

+1 for the suggestion to make parselglossy a generator.

@robertodr
Copy link
Contributor Author

Yep. It's a draft. I just forgot to mark it as such.

@bast
Copy link
Member

bast commented Jul 3, 2020

As to CLI I would find this (--target or --output, --from/--input):

$ parselglossy generate --target /my/path --from template.yml

more intuitive than this (we are not generating template.yml):

$ parselglossy generate template.yml

@robertodr
Copy link
Contributor Author

Agreed on the CLI. Will do that in the next few commits.

@robertodr robertodr force-pushed the parselglossy-as-generator branch 2 times, most recently from 42bd2fb to e3d832a Compare July 3, 2020 09:05
@robertodr robertodr marked this pull request as ready for review July 3, 2020 15:28
@robertodr
Copy link
Contributor Author

All pieces are in place and tested. It need to be documented, but the code is ready for review.

@robertodr
Copy link
Contributor Author

It's now documented 🎊 Please squash-merge. The commit messages are really bad :)

@robertodr robertodr force-pushed the parselglossy-as-generator branch 2 times, most recently from 65d1cfb to 63331a8 Compare July 5, 2020 14:18
@bast
Copy link
Member

bast commented Jul 6, 2020

I thought I could force-merge but this is disabled. Should we take out macOS builds, open an issue on that, continue work, and reintroduce them before we "release"?

@robertodr
Copy link
Contributor Author

robertodr commented Jul 6, 2020 via email

@bast
Copy link
Member

bast commented Jul 6, 2020

OK I will take care of that.

@bast bast mentioned this pull request Jul 6, 2020
Signed-off-by: Roberto Di Remigio <roberto.diremigio@gmail.com>
Signed-off-by: Roberto Di Remigio <roberto.diremigio@gmail.com>
Signed-off-by: Roberto Di Remigio <roberto.diremigio@gmail.com>
Signed-off-by: Roberto Di Remigio <roberto.diremigio@gmail.com>
Signed-off-by: Roberto Di Remigio <roberto.diremigio@gmail.com>
Signed-off-by: Roberto Di Remigio <roberto.diremigio@gmail.com>
Signed-off-by: Roberto Di Remigio <roberto.diremigio@gmail.com>
Signed-off-by: Roberto Di Remigio <roberto.diremigio@gmail.com>
Signed-off-by: Roberto Di Remigio <roberto.diremigio@gmail.com>
Signed-off-by: Roberto Di Remigio <roberto.diremigio@gmail.com>
Signed-off-by: Roberto Di Remigio <roberto.diremigio@gmail.com>
Signed-off-by: Roberto Di Remigio <roberto.diremigio@gmail.com>
Signed-off-by: Roberto Di Remigio <roberto.diremigio@gmail.com>
Signed-off-by: Roberto Di Remigio <roberto.diremigio@gmail.com>
Signed-off-by: Roberto Di Remigio <roberto.diremigio@gmail.com>
Signed-off-by: Roberto Di Remigio <roberto.diremigio@gmail.com>
Signed-off-by: Roberto Di Remigio <roberto.diremigio@gmail.com>
Signed-off-by: Roberto Di Remigio <roberto.diremigio@gmail.com>
@robertodr
Copy link
Contributor Author

They're still reported as "expected" in the checks because the main branch has them in...

@robertodr
Copy link
Contributor Author

OK, now green :)

@robertodr
Copy link
Contributor Author

robertodr commented Jul 6, 2020

Maybe you can try minting a new tag and seeing whether the deploy action actually works? I even wrote instructions on how to do it! https://parselglossy.readthedocs.io/en/latest/contributing.html#deploying

@bast
Copy link
Member

bast commented Jul 6, 2020

Still stalling on the builds. I will check whether I can give myself temporarily more merging power to get this in. The deploy action can be tested without creating tags, somehow I don't like to create tags just for testing.

@bast
Copy link
Member

bast commented Jul 6, 2020

Now it's green also on my side. Weird, some leftover. Merging.

@bast bast merged commit dd57c64 into master Jul 6, 2020
@bast bast deleted the parselglossy-as-generator branch July 6, 2020 08:01
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