Skip to content
This repository was archived by the owner on Sep 1, 2022. It is now read-only.

Display only real branch name, not "refs/heads"#17

Merged
brianamarie merged 2 commits intomasterfrom
only-display-branch
Jan 8, 2019
Merged

Display only real branch name, not "refs/heads"#17
brianamarie merged 2 commits intomasterfrom
only-display-branch

Conversation

@brianamarie
Copy link
Copy Markdown
Contributor

While going through learning lab with the participants in training this week, I saw that when opening a pull request, it shows "refs/heads" in addition to the branch name.

This pull request attempts to fix that. I haven't tested locally so I would ❤️ some review if anyone has time to get to it before I do.

cc @hectorsector @githubtraining/learning-engineering

screen shot 2018-12-03 at 17 10 49

Copy link
Copy Markdown

@ghost ghost left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues with your course have been found! You are good to go 🚀

@brianamarie brianamarie self-assigned this Dec 3, 2018
@heiskr
Copy link
Copy Markdown
Contributor

heiskr commented Dec 3, 2018

Awesome! I noticed the same thing in #11

Copy link
Copy Markdown
Contributor

@JasonEtco JasonEtco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch @brianamarie 👌 Looking at this diff, my immediate understanding is that the code is right before. Calling | replace(THING_TO_REPLACE, THING_TO_REPLACE_WITH) is what we want - that said, its obviously not doing it. I'll play around and let you know what I find out 👍

Copy link
Copy Markdown

@ghost ghost left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues with your course have been found! You are good to go 🚀

@JasonEtco
Copy link
Copy Markdown
Contributor

Since the switch to Liquid as the templating engine for responses, we can use the remove filter which does exactly what we want. I think that'll do it @brianamarie, but its worth testing to be sure.

@brianamarie
Copy link
Copy Markdown
Contributor Author

I am not sure if I will have bandwidth to test this this week, but I'd like to. @hectorsector @ppremk or other @githubtraining/learning-engineering if you have the bandwidth to test this, that would be very helpful to getting this merged 👍

@brianamarie
Copy link
Copy Markdown
Contributor Author

@hectorsector @githubtraining/learning-engineering Is there anything I can do to help, (other than test which I'm not sure how to do)?

Copy link
Copy Markdown
Contributor

@hectorsector hectorsector left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested on my local LL instance and it checks out. Check here if you'd like to see the comment.

@brianamarie
Copy link
Copy Markdown
Contributor Author

Thank you @hectorsector!

@brianamarie brianamarie merged commit 7e3d03d into master Jan 8, 2019
@brianamarie brianamarie deleted the only-display-branch branch January 8, 2019 07:08
crichID pushed a commit that referenced this pull request Feb 7, 2020
Display only real branch name, not "refs/heads"
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants