Skip to content

Add spec covering 'committer_hash'

username-removed-393947 requested to merge dandunckelman/gitlab-ce:add_spec_for_committer_hash into master

What does this MR do?

Adds a missing spec from changes added in !5822 (merged)

Are there points in the code the reviewer needs to double check?

  • Should we return if either email or name is nil?

    • This would move the logic from app/models/repository.rb#L843 to the #committer_hash function

    • This change would have #committer_hash look like this:

          def committer_hash(email:, name:)
      return if email.nil? or name.nil?

      {
        email: email,
        name: name,
        time: Time.now,
      }
    end

    • And the aforementioned code in app/models/repository.rb#L843 would become this:

          author = Gitlab::Git::committer_hash(email: email, name: name) || committer

Why was this MR needed?

In !5822 (merged), we added the #committer_hash method to lib/gitlab/git.rb, but I didn't add a spec to cover it before it was merged.

Does this MR meet the acceptance criteria?

Merge request reports