Skip to content

Improve GitHub extractor#495

Open
RensOliemans wants to merge 4 commits into
asciimoo:masterfrom
RensOliemans:github-extractor
Open

Improve GitHub extractor#495
RensOliemans wants to merge 4 commits into
asciimoo:masterfrom
RensOliemans:github-extractor

Conversation

@RensOliemans

Copy link
Copy Markdown

From #305 , I started with a stab at improving the GitHub extractor. The PR is still very much WIP, and I thought I'd create it now because I wanted to ask this question: do you want me to split github.go into separate files? Like issue.go for parsing the /owner/repo/issues/:id url, etc.

I actually have two other questions about the existing code (I'm quite new to Go):

  1. What is the purpose of Preview? Should this be unique for each type of URL as well?
  2. What was the purpose of the b string.Builder? Should I keep it, or use only metadata?

@asciimoo

Copy link
Copy Markdown
Owner

Nice!

do you want me to split github.go into separate files?

It's up to you. Having long files is idiomatic in Go if there is no real logical separation that would make sense (e.g. our server.go). Perhaps in this case it makes sense to put different handlers to different files, but it isn't necessary.

What is the purpose of Preview? Should this be unique for each type of URL as well?

Preview() produces the result preview displayed in the right pane of the results view.

What was the purpose of the b string.Builder? Should I keep it, or use only metadata?

It collects important info from the page that is assigned to the document.Text field. The advantage of assigning the content to document.Text is that it instantly becomes searchable, while the metadata fields are not searchable by default if you don't use query filtering for those fields (e.g. metadata.language:go search query).

So information that should be instantly searchable should go to document.Text (e.g. project description) and data attributes that just provide additional information in the preview (or special filtering) should go to the metadata (e.g. stars).

@asciimoo

Copy link
Copy Markdown
Owner

Let me know if I can start the review.

@RensOliemans

Copy link
Copy Markdown
Author

Thanks for the info. I'll let you know when I'm happy with it!

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