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

printCommands in interface GluegunPrint is missing the optional param… #615

Merged
merged 4 commits into from
May 5, 2020

Conversation

amartincastro
Copy link
Contributor

…ater #584

@kaihaase
Copy link

kaihaase commented Oct 3, 2019

@amartincastro I'm sorry, it's my fault.

Either the class Toolbox has to be imported: import { Toolbox } from '../domain/toolbox' or use the interface GluegunToolbox instead of the class Toolbox:

export interface GluegunPrint {
    ...
    printCommands(toolbox: GluegunToolbox, commandRoot?: string[]): void
    ...
}

More correct would probably be to import Toolbox, since the class is also used in print-tools.ts.

@jamonholmgren
Copy link
Member

@amartincastro Would you mind updating this PR? 🙏 Thank you!

@amartincastro
Copy link
Contributor Author

@amartincastro Would you mind updating this PR? 🙏 Thank you!

Hey, happy to try to help but it's been about 3 months since I looked at this, and it was literally my very first OSS contribution ever, so I am a little lost for what to do next.

My original branch is 2 commits ahead of, and 91 commits behind. Should I fork off a new version of the latest Gluegun repo and make the changes, or update the branch I originally made the PR in, then make the changes? Or something else?

@amartincastro
Copy link
Contributor Author

@amartincastro Would you mind updating this PR? 🙏 Thank you!

Hey, happy to try to help but it's been about 3 months since I looked at this, and it was literally my very first OSS contribution ever, so I am a little lost for what to do next.

My original branch is 2 commits ahead of, and 91 commits behind. Should I fork off a new version of the latest Gluegun repo and make the changes, or update the branch I originally made the PR in, then make the changes? Or something else?

I am doing Stack Overflow research to try to figure this out, but I am getting stuck on the Git workflow before even getting to the Gluegun work.

I followed the commands here to try to get my local version of the repo up-to-date so I can make the changes that @kaihaase requested on an updated version of gluegun: https://stackoverflow.com/a/7244456/6321654

But when I did, it says that Everything is up-to-date.
Screen Shot 2020-01-30 at 3 49 52 PM

I am not sure where to go next. The only thing I can think of doing is to just delete my fork, refork Gluegun, make changes as requested in this issue, and make a new PR. But I feel like there's got to be a better way. I'll wait to see what you guys say before doing that.

@amartincastro
Copy link
Contributor Author

@amartincastro Would you mind updating this PR? 🙏 Thank you!

Hey, happy to try to help but it's been about 3 months since I looked at this, and it was literally my very first OSS contribution ever, so I am a little lost for what to do next.
My original branch is 2 commits ahead of, and 91 commits behind. Should I fork off a new version of the latest Gluegun repo and make the changes, or update the branch I originally made the PR in, then make the changes? Or something else?

I am doing Stack Overflow research to try to figure this out, but I am getting stuck on the Git workflow before even getting to the Gluegun work.

I followed the commands here to try to get my local version of the repo up-to-date so I can make the changes that @kaihaase requested on an updated version of gluegun: https://stackoverflow.com/a/7244456/6321654

But when I did, it says that Everything is up-to-date.
Screen Shot 2020-01-30 at 3 49 52 PM

I am not sure where to go next. The only thing I can think of doing is to just delete my fork, refork Gluegun, make changes as requested in this issue, and make a new PR. But I feel like there's got to be a better way. I'll wait to see what you guys say before doing that.

Okay, making small strides. If you look at the screenshot, I was trying to set my own repo as the upstream, and that's why it was saying it's up-to-date. When I switched the upstream it to the infinitered/gluegun, it seemed to have updated to the latest code.

I added the import to the file, saved and did a "git push". When I pushed, it seems to have just updated the PR by itself. I am not sure what's next. Github really is challenging to understand for a newcomer.

@jamonholmgren
Copy link
Member

@amartincastro Would you want to pair on fixing this sometime?

@amartincastro
Copy link
Contributor Author

@amartincastro Would you want to pair on fixing this sometime?

That sounds good. Are you available at some point tomorrow (Fri) morning? I am wide open with the exception of 9-9:30am PT.

@jamonholmgren
Copy link
Member

@amartincastro How about Tuesday (May 5) at 10AM Pacific?

@amartincastro
Copy link
Contributor Author

Works for me. How do you prefer to screenshare?

@jamonholmgren
Copy link
Member

DM me on twitter: https://twitter.com/jamonholmgren @amartincastro

@jamonholmgren jamonholmgren merged commit 260372f into infinitered:master May 5, 2020
infinitered-circleci pushed a commit that referenced this pull request May 5, 2020
## [4.3.1](v4.3.0...v4.3.1) (2020-05-05)

### Bug Fixes

* **types:** printCommands in interface GluegunPrint is missing the optional param ([#615](#615) by [@amartincastro](https://github.com/amartincastro)) ([260372f](260372f))
@infinitered-circleci
Copy link

🎉 This PR is included in version 4.3.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

4 participants