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.

Sign up for GitHub

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

Jump to bottom

[WIP] Parser for github issues created #18

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

[WIP] Parser for github issues created #18

wants to merge 2 commits into from

Conversation

progmatic-99
Copy link

Fixes #8
Changes:

  • Parser created using regex
  • Some changes needs to be done in issue template(waiting for review by other people) to make the parser work accordingly

@progmatic-99 progmatic-99 marked this pull request as draft September 1, 2020 13:12
@progmatic-99 progmatic-99 marked this pull request as ready for review September 7, 2020 17:53
GoelJatin
GoelJatin reviewed Sep 10, 2020
View reviewed changes
Copy link
Member

@GoelJatin GoelJatin left a comment

Choose a reason for hiding this comment

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

Before we merge this, did we also try with any of the existing options for markdown parsing?

What are the challenges / issues we faced with those?

src/meetup_manager/meetup_manager/parser.py
import re


def parser(issue: str):
Copy link
Member

Choose a reason for hiding this comment

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

Did we test it works in al cases?

Very rare case, but what if a speaker intentionally leaves out a section intentionally? What happens then?

Need to add more error handling

Copy link
Author

Choose a reason for hiding this comment

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

I have tested manually on test cases that i can think of.
If speaker leaves out a section, then value corresponding to that section will be empty. I will add tests, that will clear a lot of things.

src/meetup_manager/meetup_manager/parser.py Outdated
else:
break

talk_details = json.dumps(talk_details)
Copy link
Member

Choose a reason for hiding this comment

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

why the extra step of dumping and then loading again?

Copy link
Author

Choose a reason for hiding this comment

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

I will correct this.

@progmatic-99
Copy link
Author

Before we merge this, did we also try with any of the existing options for markdown parsing?

What are the challenges / issues we faced with those?

Most of the markdown parsers that i have seen convert .md into .html which we do not need for this task, imo.
As the parser deals with a specified problem rather than a general one, it would be better to write our own parser.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@GoelJatin GoelJatin GoelJatin left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Parser for GitHub issues
2 participants
@progmatic-99 @GoelJatin