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

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

Merged
merged 1 commit into from
Jan 29, 2021
Merged

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

merged 1 commit into from
Jan 29, 2021

Conversation

jarolrod
Copy link
Member

@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.

src/qt/README.md Outdated Show resolved Hide resolved
src/qt/README.md Outdated Show resolved Hide resolved
src/qt/README.md Outdated Show resolved Hide resolved
src/qt/README.md Outdated Show resolved Hide resolved
src/qt/README.md Show resolved Hide resolved
jonasschnelli
jonasschnelli reviewed Dec 1, 2020
View reviewed changes
src/qt/README.md Outdated Show resolved Hide resolved
@jonasschnelli
Copy link
Contributor

ACK modulo 2xcomments

@RandyMcMillan
Copy link
Contributor

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

ACK :)

@RandyMcMillan
Copy link
Contributor

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
Member Author

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

src/qt/README.md Outdated Show resolved Hide resolved
src/qt/README.md Outdated Show resolved Hide resolved
src/qt/README.md Outdated Show resolved Hide resolved
src/qt/README.md Outdated Show resolved Hide resolved
src/qt/README.md Outdated Show resolved Hide resolved
src/qt/README.md Outdated Show resolved Hide resolved
src/qt/README.md Outdated Show resolved Hide resolved
src/qt/README.md Show resolved Hide resolved
src/qt/README.md Outdated Show resolved Hide resolved
src/qt/README.md Outdated Show resolved Hide resolved
@jarolrod
Copy link
Member Author

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

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.

src/qt/README.md Outdated Show resolved Hide resolved
src/qt/README.md Outdated Show resolved Hide resolved
src/qt/README.md Outdated Show resolved Hide resolved
src/qt/README.md Show resolved Hide resolved
@jarolrod
Copy link
Member Author

updated a9e641d -> 63d0f29, addressed @jonatack suggestions

jonatack
jonatack reviewed Jan 22, 2021
View reviewed changes
src/qt/README.md Outdated Show resolved Hide resolved
@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
Member Author

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

@jonasschnelli jonasschnelli jonasschnelli left review comments

@RandyMcMillan RandyMcMillan RandyMcMillan left review comments

@jonatack jonatack jonatack approved these changes

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