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

Removed unnecessary string replacements #2635

Closed
wants to merge 1 commit into from

Conversation

RichardLitt
Copy link
Contributor

If .html is removed from the end only to be added to the end, there's no reason to touch it. Also, template + / is the same as template/.

I haven't tested this, but I see absolutely no reason why this code isn't the same functionally.

@chrisirhc
Copy link
Contributor

This looks like a documentation change and should be under docs commit type.

If `.html` is removed from the end only to be added to the end, there's no reason to touch it. Also, `template` + `/` is the same as `template/`.

I haven't tested this, but I see absolutely no reason why this code isn't the same functionally.
@RichardLitt
Copy link
Contributor Author

That should be unnecessary. This was a commit filed through the GitHub interface; it's a massive barrier to entry if people can't use the GitHub inline editor. You can tell it is a GitHub-originating fix by the branch name.

Cf. third paragraph of the guidelines.

@RichardLitt
Copy link
Contributor Author

*can't use it as is, without reading the contributing guidelines to figure out how to style a commit message. Just sayin'.

@chrisirhc
Copy link
Contributor

I see, I didn't realize that the fix was from Github's inline editor. I'll be sure to check that next time.

Changing the commit message first before merging it into master isn't a whole lot of work for me, I just wanted to communicate the convention to contributors.

@RichardLitt
Copy link
Contributor Author

No worries. The docs about contribution on this point aren't in fact clear. I'll add something about that for now, this whole process should be easier.

RichardLitt added a commit to angular/angular.js that referenced this pull request Sep 29, 2014
It's important that we let people use the GitHub editing interface without being 100% strict about how to name the commit changes. Otherwise, it is basically a barrier to entry and highly discouraging for new people who may just be trying to fix a spelling error. Since it is possible for contributors to edit the commit message before merging it into master, for people who are new to the commit styling system, we should be lenient about minor infractions like forgetting to put docs: in front of a message. 

CF: angular-ui/bootstrap#2635 (comment)
@chrisirhc chrisirhc closed this in 4fc68b8 Oct 1, 2014
@chrisirhc
Copy link
Contributor

Thank you!

@RichardLitt
Copy link
Contributor Author

👍

ulle pushed a commit to ulle/bootstrap that referenced this pull request Oct 22, 2014
If `.html` is removed from the end only to be added to the end, there's no reason to touch it. Also, `template` + `/` is the same as `template/`.

I haven't tested this, but I see absolutely no reason why this code isn't the same functionally.

Closes angular-ui#2635
OronNadiv pushed a commit to lanetix/bootstrap that referenced this pull request Nov 18, 2014
If `.html` is removed from the end only to be added to the end, there's no reason to touch it. Also, `template` + `/` is the same as `template/`.

I haven't tested this, but I see absolutely no reason why this code isn't the same functionally.

Closes angular-ui#2635
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.

2 participants