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.

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

Fixes for issues 158,159,160,161,162,163, improved local and remote tests #164

Closed
wants to merge 4 commits into from
Closed

Conversation

kurtkanaskie
Copy link
Contributor

Fixed bugs and updated tests.
Remote tests still need some work, but works fine for newly added features.
npm test
npm run remotetest

@whitlockjc
Copy link
Member

I hate to request this but it would be really nice if this PR were split up into separate PRs per issue/feature (where possible). This makes it much easier to trace changes, rollback things (if needed), etc.

@kurtkanaskie
Copy link
Contributor Author

I was afraid of that, I found bugs while building new features and just fixed them and added Issues for tracking. If you want, I can separate into individual PRs. Do I have to wait for each merge or can I stack up PRs, one per issue?

@whitlockjc
Copy link
Member

Stack them and I'll merge in order. I apologize for asking for this but I think it makes sense. As for fixing bugs in a PR, I'm fine with that. Title your commit based on what you're doing, and then add a note into the commit referring to fixing a bug. Example:

Added support for ...

* This commit also includes a bug fix found while doing the work, the
bug fixed is ...

This is just an example, format and word however you want.

@kurtkanaskie
Copy link
Contributor Author

OK, how about this, I'll fix the latest issue #165 "remotetests" and add in bug fixes for #158 #160 #161 and #163 all related to options. Then I'll do #162 then #159
So 3 PRs in total, does that work?

@whitlockjc
Copy link
Member

Sounds good.

@kurtkanaskie
Copy link
Contributor Author

Closing and redoing incremental PRs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants