Skip to content

Proposed changes for enhancement:#314

Closed
Wind010 wants to merge 7 commits into
commandlineparser:developfrom
Wind010:master
Closed

Proposed changes for enhancement:#314
Wind010 wants to merge 7 commits into
commandlineparser:developfrom
Wind010:master

Conversation

@Wind010

@Wind010 Wind010 commented Aug 14, 2018

Copy link
Copy Markdown

@ericnewton76 ericnewton76 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I might roll back some of the CSPROJ reference changes made here...

We switched to being a NetCore library now.

@Wind010

Wind010 commented Jan 18, 2019

Copy link
Copy Markdown
Author

Sounds good to me.

@Wind010

Wind010 commented Jan 18, 2019

Copy link
Copy Markdown
Author

Should I just pull latest into my fork and resolve the conflicts?

@ericnewton76

ericnewton76 commented Jan 18, 2019

Copy link
Copy Markdown
Member

That would be easiest for me... when I'm merging, it's hard to discern your code from somebody else's

When you are merging its much easier to discern your code from somebody else's ;-)

^ ^ ^ ^
Yeah, when you first read it, its weird and then it makes sense... lol

@Wind010

Wind010 commented Jan 24, 2019

Copy link
Copy Markdown
Author

I've merged down to my fork and resolved the conflicts. Basically took everything from upstream and just kept my changes.

@ericnewton76

Copy link
Copy Markdown
Member

Looks like there was a csproj merge issue in the appveyor build

@ericnewton76 ericnewton76 changed the base branch from master to develop January 27, 2019 17:48
@Wind010

Wind010 commented Jan 29, 2019

Copy link
Copy Markdown
Author

Build fixed locally, updating tests.

@Wind010

Wind010 commented Jan 30, 2019

Copy link
Copy Markdown
Author

Please review latest changes.

@twaalewijn

Copy link
Copy Markdown

@Wind010 Excuse me for asking and perhaps I am missing something but could it be that your merge commit already accidentally ended up in the main repository?

I did a fresh fork a couple of days ago and building the develop/master branches gives me errors that would have been fixed by your latest commit.

@Wind010

Wind010 commented Feb 23, 2019

Copy link
Copy Markdown
Author

@twaalewijn Yeah, it looks like the initial PR changes were merged into the main repository:
bd67c24

I've merged merged upstream master into my fork's master and resolved the breaks. All tests are successful in the latest changes in this PR.

@Wind010

Wind010 commented Feb 23, 2019

Copy link
Copy Markdown
Author

@ericnewton76 Should I close this PR and open one where I merge from my fork master to the upstream master?

@ericnewton76

Copy link
Copy Markdown
Member

thats odd, seems like it should still pickup your changes.

Did you check into your Wind010/master branch?

@Wind010

Wind010 commented Mar 4, 2019

Copy link
Copy Markdown
Author

Sorry for the late reply. Yes, master should have the latest changes with the conflicts resolved.

https://github.com/Wind010/commandline/commits/master

The broken commit was made on the 24th, it looks like the merge of my fork to main fork was done on the 27th, but the fix/proper resolve was done on the 29th.

@ebjornset

ebjornset commented Mar 24, 2019

Copy link
Copy Markdown

@Wind010, @ericnewton76 I just forked commandline, and it seems to me that this PR is not merged in yet? At least I get compilation errors for the issues this PR seems to fix.

@moh-hassan

Copy link
Copy Markdown
Collaborator

@ebjornset
Yes, You are correct.
There is a compilation error in the commit 030eb8d on Feb 1.
Last merge need to be corrected.

@Wind010, @ericnewton76
It seems that last merge cause a compilation error and need to be fixed.
I had to stop rebasing my PR with the upstream

@Wind010

Wind010 commented Apr 8, 2019

Copy link
Copy Markdown
Author

I'll open up a fresh PR to merge from commandlineparser:develop from wind010:master.

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.

5 participants