Skip to content
Snippets Groups Projects

Rewrite build_url, use web_protocol for repo indicator.

Merged gitlab-qa-bot requested to merge github/fork/dosire/repo_clone_indicator into master

Created by: dosire

Display HTTPS as the repo clone indicator when that is the protocol.

Had to rewrite the build_url function since it was modifying the web_protocol variable.

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
Unable to load the diff
  • Created by: dzaporozhets

    i afraid we'll get undefined method custom_port at line 39 if web_custom_port? is false

    By Administrator on 2012-10-21T13:48:45 (imported from GitLab project)

    By Administrator on 2012-10-21T13:48:45 (imported from GitLab)

  • gitlab-qa-bot
  • Unable to load the diff
    • Created by: dosire

      @randx Your concern is understandable, thank you for sharing it. However, the local variable custom_port will be initialized (with the value nil) even if web_custom_port? is false or nil. From http://ruby.runpaint.org/variables about local variables: "It is initialized if it appears on the left‐hand side (before the equals sign (U+003D)) of an assignment expression, even if the expression does not actually execute. Variables of the latter sort have the value nil."

      By Administrator on 2012-10-21T13:48:45 (imported from GitLab project)

      By Administrator on 2012-10-21T13:48:45 (imported from GitLab)

  • gitlab-qa-bot
  • Unable to load the diff
    • Created by: dzaporozhets

      thanks. I dont know it. But lets write simple code :)

      By Administrator on 2012-10-21T13:48:45 (imported from GitLab project)

      By Administrator on 2012-10-21T13:48:45 (imported from GitLab)

  • gitlab-qa-bot
  • Unable to load the diff
    • Created by: dosire

      @randx Ok, do you want me to replace it with the following?

      custom_port = if web_custom_port?
        web_port
      else
        nil
      end

      By Administrator on 2012-10-21T13:48:45 (imported from GitLab project)

      By Administrator on 2012-10-21T13:48:45 (imported from GitLab)

  • gitlab-qa-bot
  • Unable to load the diff
    • Created by: dosire

      @randx Maybe you can let me know what you think about the suggestion above. We are running into a bug with the existing code. raw_url = self.web_protocol makes raw_url the same as self.web_protocol, when you modify raw_url after that self.web_protocol is also changed. This leads to errors such as https://github.com/dosire/gitlabhq/issues/58. You could either use raw_url = self.web_protocol.dup or merge this PR to solve it.

      By Administrator on 2012-10-21T13:48:45 (imported from GitLab project)

      By Administrator on 2012-10-21T13:48:45 (imported from GitLab)

  • Created by: dosire

    @randx After our discussion I adapted the code to make the initialization obvious. Please pull this or give feedback since it fixes a bug.

    By Administrator on 2012-10-18T14:04:31 (imported from GitLab project)

    By Administrator on 2012-10-18T14:04:31 (imported from GitLab)

  • Created by: dzaporozhets

    Rebase please so i can merge it

    By Administrator on 2012-10-21T07:31:31 (imported from GitLab project)

    By Administrator on 2012-10-21T07:31:31 (imported from GitLab)

  • Created by: dosire

    @randx I rebased it.

    By Administrator on 2012-10-21T11:48:27 (imported from GitLab project)

    By Administrator on 2012-10-21T11:48:27 (imported from GitLab)

  • Created by: dzaporozhets

    @dosire still unmergable :)

    By Administrator on 2012-10-21T13:12:32 (imported from GitLab project)

    By Administrator on 2012-10-21T13:12:32 (imported from GitLab)

  • Created by: dosire

    @randx Sorry, I can't see the merge status in this PR. I merged in the master again and tested with a PR on my own repo and it looks green: https://github.com/dosire/gitlabhq/pull/62 and https://travis-ci.org/#!/gitlabhq/gitlabhq/builds/2875236 I hope everything is ok now. If not let me know.

    By Administrator on 2012-10-21T14:03:49 (imported from GitLab project)

    By Administrator on 2012-10-21T14:03:49 (imported from GitLab)

  • Created by: dosire

    @randx Thanks! Really glad this got in for 3.0 :zap:

    By Administrator on 2012-10-22T10:41:28 (imported from GitLab project)

    By Administrator on 2012-10-22T10:41:28 (imported from GitLab)

  • Created by: dzaporozhets

    @dosire np :). Release in few hours

    By Administrator on 2012-10-22T12:54:20 (imported from GitLab project)

    By Administrator on 2012-10-22T12:54:20 (imported from GitLab)

  • Created by: dosire

    @randx Great, we're looking forward to it :-)

    By Administrator on 2012-10-22T13:09:32 (imported from GitLab project)

    By Administrator on 2012-10-22T13:09:32 (imported from GitLab)

  • Please register or sign in to reply
    Loading