-
Notifications
You must be signed in to change notification settings - Fork 0
[#250] Full refactoring (Draft) #271
base: develop
Are you sure you want to change the base?
Conversation
package codesearch.core.index.indexer | ||
|
||
import cats.effect.{ContextShift, Sync} | ||
import codesearch.core.index.directory.СindexDirectory.HaskellCindex |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please fix the Cyrillic C in "CindexDirectory" while you're at it, and all other Cs in the codebase.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@neongreen Thanks for finding mistake.
@@ -19,27 +19,35 @@ db { | |||
|
|||
languagesConfig { | |||
haskell { | |||
repository = "hackage" | |||
repoIndexUrl = "http://hackage.haskell.org/packages/index.tar.gz" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you change this to https?
repoArchivePath = "./data/meta/ruby/ruby_index.gz" | ||
repoJsonPath = "./data/meta/ruby/ruby_index.json" | ||
scriptPath = "./scripts/update_index.rb" | ||
concurrentTasksCount = 30 | ||
} | ||
javascript { | ||
repository = "npm" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this a tab? Please no tabs
def upsert(name: String, version: String, repository: String): F[Int] = { | ||
sql""" | ||
INSERT INTO package(name, version, repository, updated_at) | ||
VALUES ($name, $version, $repository, now()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will this escape name
? Because otherwise we might get an SQL injection.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I will check it.
repoArchivePath = "./data/meta/rust/archive.zip" | ||
repoPath = "./data/meta/rust/" | ||
concurrentTasksCount = 30 | ||
ignoreFiles = ["test-max-version-example-crate", "version-length-checking-is-overrated", "config.json", "archive.zip", ".git"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is config.json excluded? (Not saying it shouldn't be, just asking why)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@neongreen I can't answer why is so =( but it was originally. I will remove it, thanks.
def batchUpsert(packages: List[PackageIndexTableRow]): F[Int] = { | ||
Update[PackageIndexTableRow]( | ||
""" | ||
|INSERT INTO repository_index(name, version, repository) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What are the pipes doing here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@neongreen it's Scala multi-row strings definition syntax.
Now this PR is WIP. Not compiling.