Skip to content
This repository has been archived by the owner on Jun 4, 2019. It is now read-only.

Links to ruby source from details page are not working #39

Open
jmccaffrey opened this issue Aug 7, 2015 · 10 comments
Open

Links to ruby source from details page are not working #39

jmccaffrey opened this issue Aug 7, 2015 · 10 comments

Comments

@jmccaffrey
Copy link

Example:
http://www.docsdoctor.org/doc_methods/25285
the Source link points to
https://github.com/ruby/ruby/blob/master/ext/json/lib/json/add/regexp.rb/#L11
which returns 404
It would appear that the ruby project doesn't have a 'master' branch, but instead they have 'trunk'
so a link like this would work
https://github.com/ruby/ruby/blob/trunk/ext/json/lib/json/add/regexp.rb/#L11
This 404 result seemed consistent for all of the ruby methods I looked at (which would make sense)

Within docs_doctor, it looks like this code is responsible for building the link https://github.com/codetriage/docs_doctor/blob/master/app/models/github_url_from_base_path_line.rb

 # https://github.com/rails/rails/blob/master/actionmailer/lib/action_mailer/collector.rb#L10
  def to_github
    File.join(@base, "blob/master", @path, "#L#{@line}")
  end

I don't know the best way to resolve this, but perhaps something where you can specify the 'branch' url section to use for the repo. ("blob/master" would be the default, but 'blob/trunk' would be specified for the ruby repo.

@prathamesh-sonpatki
Copy link
Member

@jmccaffrey Github returns the default branch of repo in the response. For eg. https://api.github.com/repos/ruby/ruby returns default_branch as trunk in the JSON response. We should use that information and store it in our database. Would you like to work on this issue?

@jmccaffrey
Copy link
Author

It would probably take me half a day to get oriented in this project and make the changes and update the tests, so it is probably faster for someone else to do it.

Here's what I got so far:

fix urls
  migration
    add column to repo table
      default_branch  # length?
       # default value of 'master'  ?? 
    repo.rb
     repo#update_from_github
      save default_branch into repo

    in github_url_from_base_path_line.rb
      allow default_branch to be passed in
      include it in #to_github method

      where GithubUrlFromBasePathLine is called
        https://github.com/codetriage/docs_doctor/search?utf8=%E2%9C%93&q=GithubUrlFromBasePathLine

        update calls to GithubUrlFromBasePathLine, to pass in default_branch value
        mailer
        show page
        readme

I don't see where Repos are loaded/created.
I'm not sure how you manage deploying a column add, and code that depends on it. (eg. deploy the db migration, but not the code that depends on it yet. Run a script to populate the existing repos table rows. Then deploy the other code that expects the column to be there).

I wouldn't mind pairing on it, but I feel like I'd take too long, and break too much stuff on my own!

@jmccaffrey
Copy link
Author

Seems like this is still broken.
A better title might be "All links to source are broken for repos that don't have a Master branch", as the issue might be bigger than just the ruby repo.

Can we sketch out the next steps for how this could be resolved? (I listed out some thoughts above, but I have no idea if I'm even close)

@schneems
Copy link
Member

We should use urls like this (i'm thinking)

https://github.com/codetriage/docs_doctor/blob/f5dd91595d5cdad0a2348515c6f715ef19c51070/app/models/github_url_from_base_path_line.rb#L10

So it's base

https://github.com/codetriage/docs_doctor/blob

then commit sha

f5dd91595d5cdad0a2348515c6f715ef19c51070

Then page and line

/app/models/github_url_from_base_path_line.rb#L10

@kddnewton
Copy link
Contributor

Hi everyone - I like the project! Added a PR to fix this issue.

@kddnewton
Copy link
Contributor

I think we can close this ticket now. Still working on figuring out why the rails links aren't exactly correct. The ruby ones are working though.

@nateberkopec
Copy link

Yeah, just got an email for rails - 100% of the source links were wrong :(

@kddnewton
Copy link
Contributor

Yeah we should close this one and open a new one because the ruby source ones work now but the rails ones are broken, and I'm pretty sure it's for a different reason.

@prathamesh-sonpatki
Copy link
Member

@nateberkopec Did you notice any issue with links other than Rails?

@prathamesh-sonpatki
Copy link
Member

Created new issue for tracking problems with Rails inks #45

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants