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

Add support for Phabricator URLs #54

Merged
merged 2 commits into from Dec 9, 2018

Conversation

Projects
None yet
2 participants
@kuba-orlik
Copy link

kuba-orlik commented Dec 2, 2018

Had some issues with generating a commit URL, because Phabricator uses reposiotories' callsigns for that and those aren't included in the repository URL. In the end I've decided to just ask the user for the callsign when generating a commit URL. File/region URLs work just fine

@rmuslimov

This comment has been minimized.

Copy link
Owner

rmuslimov commented Dec 7, 2018

I'm really sure how test phabricator integration and solve callsign issue. However I know that that projects usually have .arcconfig file and it has information about CALLSIGN. Is it a good idea to try pull it from there?

@kuba-orlik

This comment has been minimized.

Copy link

kuba-orlik commented Dec 9, 2018

Our team has multiple repositories on Phabricator and not all of them have an .arcconfig file. And in those .arcconfig files there's no information about the callsign...

@kuba-orlik

This comment has been minimized.

Copy link

kuba-orlik commented Dec 9, 2018

I've asked the Phabricator folks for help, maybe they'll help :)

https://discourse.phabricator-community.org/t/get-web-view-url-to-a-commit-without-knowing-the-callsign/2177

Do you think that the PR is mergeable in the current state? Personally I use mostly just the browse-file functionality, anyway

@rmuslimov rmuslimov merged commit 9f91bb5 into rmuslimov:master Dec 9, 2018

@rmuslimov

This comment has been minimized.

Copy link
Owner

rmuslimov commented Dec 9, 2018

Yeah, definitely! Thanks for adding phab!

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