codetriage / docs_doctor

http://docsdoctor.org
27 stars 21 forks source link

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

Open jmccaffrey opened 9 years ago

jmccaffrey commented 9 years ago

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 commented 9 years ago

@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 commented 9 years ago

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 commented 9 years ago

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 commented 9 years ago

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 commented 8 years ago

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

kddnewton commented 8 years ago

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 commented 8 years ago

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

kddnewton commented 8 years ago

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 commented 8 years ago

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

prathamesh-sonpatki commented 8 years ago

Created new issue for tracking problems with Rails inks https://github.com/codetriage/docs_doctor/issues/45