Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Adds pronto-tslint_npm to list of runners in README.md #245

Merged
merged 1 commit into from
Jun 10, 2017
Merged

Adds pronto-tslint_npm to list of runners in README.md #245

merged 1 commit into from
Jun 10, 2017

Conversation

eprislac
Copy link
Contributor

@eprislac eprislac commented Jun 9, 2017 •
edited

Adds pronto-tslint_npm to the list of runners in README.md

@eprislac eprislac changed the title Squashed commit of the following: Adds pronto-tslint_npm to list of runners in README.md Jun 9, 2017
@ivanovaleksey
Copy link
Contributor

@eprislac now your commit has very strange name.
Why did you create a new branch?
You could just squash commits and force push to the old branch.

Adds pronto-tslint_npm to list of runners in README.md
cce7d1f 
commit d5d4048
Author: Eddie Prislac <[email protected]>
Date:   Thu Jun 8 13:04:03 2017 -0500

    Remove unneccesary file

commit 3616b2e
Author: Eddie Prislac <[email protected]>
Date:   Thu Jun 8 13:00:56 2017 -0500

    Patch

commit f49f293
Author: Eddie Prislac <[email protected]>
Date:   Thu Jun 8 12:49:18 2017 -0500

    Add pronto-tslint_npm to runner list in README.md
@eprislac eprislac force-pushed the add_pronto_tslint_npm_to_readme_2 branch from 5bdef7a to cce7d1f Compare June 9, 2017 14:09
@eprislac
Copy link
Contributor Author

eprislac commented Jun 9, 2017

@ivanovaleksey I'm unaware of SOP here, but force-pushing to branches that have already been published is usually considered a bad practice, so didn't think to do so. Also, I've no experience squashing commits outside of merging (The weird commit message is actually the default message you get when performing a squash-merge in git). I've amended the commit message and pushed it back up, is this satisfactory for listing my gem?

@ivanovaleksey
Copy link
Contributor

I believe force pushing to your own small-fix-branch is totally fine because no one didn't fork from this branch.
I am not a judge here, but now it looks better IMO.

P. S.
You can squash commits with interactive rebase.

@eprislac
Copy link
Contributor Author

eprislac commented Jun 9, 2017 •
edited

Thanks for the link on interactive-rebase. On the job, I'm used to working on projects where we handle merges and PRs very differently, (we work from branches of a cloned repo, rather than forks, and most history is preserved on those branches - we then squash the commits when merging), so appreciate the guidance on how you guys do things here. If you're doing a PR, it's your job to judge, so I'm not offended.

@mmozuras
Copy link
Member

@eprislac thanks for the contribution and writing a runner! 🙇

@ivanovaleksey thanks for reviewing this PR! 😄

@mmozuras mmozuras merged commit 3974c6b into prontolabs:master Jun 10, 2017
apiology pushed a commit to apiology/pronto that referenced this pull request Dec 27, 2019
@mmozuras
Merge pull request prontolabs#245 from eprislac/add_pronto_tslint_npm…
7b71815 
…_to_readme_2

Adds pronto-tslint_npm to list of runners in README.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@eprislac @ivanovaleksey @mmozuras