Skip to content

doc: Improve gui/src/qt README.md #139

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
Merged
maflcko merged 1 commit into from
Jan 29, 2021
Merged

doc: Improve gui/src/qt README.md #139

maflcko merged 1 commit into from
Jan 29, 2021

Conversation

@jarolrod
Copy link
Contributor

@jarolrod jarolrod commented Nov 28, 2020 •
edited
Loading

Master/Before: Render of Master

PR/After: Render of PR

Changes:
The README.md found in gui/src/qt seems to not have gotten any love in a while. This PR fixes some grammatical errors, makes it easier to follow, and modernizes the logic of using Qt Creator.

  1. Makes several sections more informative
  2. Directories under Files and Directories now end with a forward slash denoting that they are a directory
  3. Modernize the Qt Creator Logic for the current setup flow
  4. Add UNIX Qt Creator Setup Instructions (Ubuntu & Debian)

@fanquake fanquake added the Doc label Nov 28, 2020
hebasto
hebasto reviewed Nov 28, 2020
View reviewed changes
Copy link
Member

@hebasto hebasto left a comment

Choose a reason for hiding this comment

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

For editing *.ui files the Qt Designer is enough for me, but this opinion is personal, ofc.

jonasschnelli
jonasschnelli reviewed Dec 1, 2020
View reviewed changes
jonasschnelli
jonasschnelli reviewed Dec 1, 2020
View reviewed changes
@jonasschnelli
Copy link
Contributor

jonasschnelli commented Dec 1, 2020

ACK modulo 2xcomments

@RandyMcMillan
Copy link
Contributor

RandyMcMillan commented Dec 2, 2020

@jarolrod - You should be able to trigger a rebuild yourself.
https://cirrus-ci.com/task/6120245750398976

ACK :)

@RandyMcMillan
Copy link
Contributor

RandyMcMillan commented Dec 4, 2020

The parent commits never passed CI

The current parent commits

ca759bc
62c93c3
b40d4d4

I suggest rebasing it to

a0489f3

As you can see - it passed CIRRUS

https://cirrus-ci.com/build/4908623950249984

and travis-ci

https://travis-ci.org/github/bitcoin-core/gui/builds/747420647

@RandyMcMillan RandyMcMillan mentioned this pull request Dec 4, 2020
Closed
@jarolrod jarolrod force-pushed the improve-qt-readme branch 3 times, most recently from af8c5a5 to f6584d9 Compare January 15, 2021 02:46
@jarolrod
Copy link
Contributor Author

jarolrod commented Jan 15, 2021

updated to f6584d9

Addressed @hebasto comments and @jonasschnelli suggestions

jonatack
jonatack reviewed Jan 15, 2021
View reviewed changes
Copy link
Member

@jonatack jonatack left a comment

Choose a reason for hiding this comment

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

Concept ACK

@jarolrod
Copy link
Contributor Author

jarolrod commented Jan 16, 2021

updated f6584d9 -> a9e641d

Implemented @jonatack suggestions. Thanks for the in-depth review!

Additionally, I have included some Qt Creator setup instructions for UNIX (Ubuntu & Debian). Windows instructions still need to be done. I will take care of that in a follow-up PR.

New Render: src/qt/README.md

@jonatack
Copy link
Member

jonatack commented Jan 16, 2021

Thanks, will re-review.

jonatack
jonatack reviewed Jan 21, 2021
View reviewed changes
Copy link
Member

@jonatack jonatack left a comment

Choose a reason for hiding this comment

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

Almost-ACK, just a couple more items below.

@jarolrod
Copy link
Contributor Author

jarolrod commented Jan 22, 2021

updated a9e641d -> 63d0f29, addressed @jonatack suggestions

jonatack
jonatack reviewed Jan 22, 2021
View reviewed changes
@jarolrod
Improve gui/src/qt README.md
5d1f260 
The current readme is a little bit outdated and contains some grammatical mistakes. This commit updates the doc so that:
  - It is easier to follow and is more informative
  - Fixes grammatical mistakes
  - Modernizes the Qt Creater setup instructions
  - Adds UNIX instructions for Qt Creator setup
@jarolrod
Copy link
Contributor Author

jarolrod commented Jan 28, 2021

updated 63d0f29 -> 5d1f260, addressed leading whitespace issue

jonatack
jonatack approved these changes Jan 28, 2021
View reviewed changes
Copy link
Member

@jonatack jonatack left a comment

Choose a reason for hiding this comment

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

ACK 5d1f260

@RandyMcMillan
Copy link
Contributor

RandyMcMillan commented Jan 28, 2021 •
edited
Loading

ACK 5d1f260 👍🏼

@maflcko maflcko merged commit bc5f26d into bitcoin-core:master Jan 29, 2021
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Jan 29, 2021
Merge bitcoin-core/gui#139: doc: Improve gui/src/qt README.md
41af9dd
@jarolrod jarolrod deleted the improve-qt-readme branch January 29, 2021 17:40
@bitcoin-core bitcoin-core locked as resolved and limited conversation to collaborators Aug 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@hebasto hebasto hebasto left review comments

+3 more reviewers

@RandyMcMillan RandyMcMillan RandyMcMillan left review comments

@jonasschnelli jonasschnelli jonasschnelli left review comments

@jonatack jonatack jonatack approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Doc

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@jarolrod @jonasschnelli @RandyMcMillan @jonatack @hebasto @fanquake @maflcko