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

Editline fix #1949

Merged
merged 2 commits into from
Sep 1, 2019
Merged

Editline fix #1949

merged 2 commits into from
Sep 1, 2019

Conversation

pmconrad
Copy link
Contributor

@pmconrad pmconrad commented Aug 21, 2019 •
edited by abitmore
Loading

Fix #1948 in release and fix #1952 by bumping fc.
Note the bump is not to fc/master but only includes the fix.

@pmconrad pmconrad added this to the 3.3.0 - Feature Release milestone Aug 21, 2019
@abitmore
Copy link
Member

Created branch for-core-3-3 in FC to hold commits for 3.3 release. In case when there are new changes for 3.3, please create PRs against this branch.

@abitmore
Copy link
Member

Note this will introduce a conflict when merging release back to develop due to #1946, it's unavoidable though. Not a big deal anyway.

abitmore
abitmore previously requested changes Aug 21, 2019
View reviewed changes
Copy link
Member

@abitmore abitmore left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Editline is not available in MinGW. I guess this fix will break MinGW build.

@abitmore abitmore dismissed their stale review August 21, 2019 15:39

It seems that Editline is now available in MinGW

@abitmore
Copy link
Member

abitmore commented Aug 21, 2019 •
edited
Loading

I'm confused. In bitshares/bitshares-fc#125 editline for MinGW got removed (it's in release notes of 3.2.0). In bitshares/bitshares-fc#121 editline for MinGW is re-added with no comment, is it correct? @crypto-ape please check.

More info in issue #1724.

@ryanRfox
Copy link
Contributor

Also desire for @cwyyprog to review/comment on this issue related to Editline now available in MinGW. It seems you are familiar with building with MinGW. Are you able to build successfully and include Editline?

@ryanRfox
Copy link
Contributor

@crypto-ape can you reach me on TG or WC? I use the same username as GH on those as well. Thanks.

@MichelSantos MichelSantos mentioned this pull request Aug 21, 2019
Closed
@abitmore abitmore mentioned this pull request Aug 21, 2019
Closed
3 tasks
@pmconrad
Copy link
Contributor Author

In bitshares/bitshares-fc#121 editline for MinGW is re-added with no comment,

Not really. Editline is in/excluded here depending on the WIN32 flag which is true also for mingw builds. It is not possible to build editline with mingw.

Note: please keep this PR open for now, we may need to bump fc again for #1952

@crypto-ape
Copy link
Contributor

Thanks @pmconrad, that is exactly it.

Before, there was a mismatch between the use of MSVC and WIN32 cases.

As @pmconrad commented, MINGW implies WIN32 so the bitshares/bitshares-fc#121 should be ok.

IIRC I have compiled it successfully with MinGW on both Linux and Windows.

@abitmore
Copy link
Member

Thanks for the clarification.

@ryanRfox
Copy link
Contributor

Do we consider this one "done" @abitmore @pmconrad ? We have #1947 pending this one. Thanks

@abitmore
Copy link
Member

@ryanRfox no, we wanted to (ab)use this PR to bump FC for #1952, so will add more commit(s) to this PR.

@pmconrad pmconrad merged commit 9942f79 into bitshares:release Sep 1, 2019
@pmconrad pmconrad deleted the 1948_editline_fix branch September 1, 2019 07:52
@pmconrad pmconrad mentioned this pull request Sep 1, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@abitmore abitmore abitmore approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
3.3.0 - Feature Release
Development

Successfully merging this pull request may close these issues.
4 participants
@pmconrad @abitmore @ryanRfox @crypto-ape