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

cmd/server/commands: use relative path as id instead of last part #816

Merged
merged 2 commits into from
May 20, 2019

Conversation

erizocosmico
Copy link
Contributor

@erizocosmico erizocosmico commented May 17, 2019

Fixes #681

Still can have collisions, but reduces the probability of having them.

Signed-off-by: Miguel Molina miguel@erizocosmi.co

Fixes src-d#681

Signed-off-by: Miguel Molina <miguel@erizocosmi.co>
@erizocosmico erizocosmico requested a review from a team May 17, 2019 12:19
Copy link
Contributor

@kuba-- kuba-- left a comment

Choose a reason for hiding this comment

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

The only thing what I can think right now is if we can internally keep paths in a unix format, so always convert \ to / and if you have a windows HDD prefix, e.g.: C:\ just convert it to: /c/
WDYT?

@erizocosmico
Copy link
Contributor Author

@kuba-- why would you want to store all paths in unix format?

@kuba--
Copy link
Contributor

kuba-- commented May 17, 2019

@erizocosmico - just to have it internally consistent (like you start gitbase on some OS and query on another). Like you have inside a zip file - all entries have unix separator(/) even if you use windows or pass \

@erizocosmico
Copy link
Contributor Author

erizocosmico commented May 17, 2019 via email

@juanjux
Copy link
Contributor

juanjux commented May 17, 2019

Windows 10 will accept / and \ as path separators, don't know about previous versions, anyway unifying doesn't hurt.

Signed-off-by: Miguel Molina <miguel@erizocosmi.co>
@ajnavarro ajnavarro merged commit 7a1da95 into src-d:master May 20, 2019
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.

Use full path as repository_id
4 participants