Skip to content

contrib: remove no longer used contrib/bitcoin-qt.pro #21695

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
fanquake merged 1 commit into from
Apr 16, 2021
Merged

contrib: remove no longer used contrib/bitcoin-qt.pro #21695

fanquake merged 1 commit into from
Apr 16, 2021
+0 −24

Conversation

@hebasto
Copy link
Member

@hebasto hebasto commented Apr 15, 2021

From IRC:

<hebasto> wumpus: I cannot see any way how the contrib/bitcoin-qt.pro is used in the translation process, neither in the main repo nor in https://github.com/bitcoin-core/bitcoin-maintainer-tools. Besides it looks outdated and unmaintained. May I ask you to confirm/deny my assumption?
<wumpus> hebasto: it is not used for anything, it exists to be able to edit the qt forms in qt designer nothing more
<wumpus> i'm not sure if it is even necessary for that, but it is why it is there
<hebasto> wumpus: thanks, qt designer does not need *.pro file at all
<hebasto> maybe qt creator does
<wumpus> feel free to create a PR to remove it, best way to find out if someone wants to keep it, you are right it hasn't been updated in a long time
<hebasto> ok
<wumpus> fwiw, the only question i get about it ever is why it exists
<hebasto> it was in use with qmake years ago (what I found digging into the repo history)
<wumpus> yes, that was the original reason, but when we switched to automake it was kept around for use w/ qt's GUI tools
<hebasto> I've noticed it in https://github.com/bitcoin/bitcoin/blame/master/doc/translation_process.md#L25
<wumpus> what it says there is definitely not true anymore

@laanwj
Copy link
Member

laanwj commented Apr 15, 2021

ACK 5f2be6e

@hebasto
Copy link
Member Author

hebasto commented Apr 15, 2021

@jarolrod

As you was the last person who checked Using Qt Creator as an IDE, could your confirm/deny the necessary of the contrib/bitcoin-qt.pro for such setup?

jarolrod
jarolrod reviewed Apr 15, 2021
View reviewed changes
Copy link
Contributor

@jarolrod jarolrod left a comment

Choose a reason for hiding this comment

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

ACK on removing the line about bitcoin-qt.pro from doc/translation_process.md.

The bitcoin-qt.pro file is a convenient way to quickly open up the GUI ui files and start editing them. The flow is (Using Qt Creator) Open Project -> Select pro file -> Edit UI Files. This can be the recommended flow to suggest to designers who'd like to make UI changes. I think it should stay for this reason, and I can add a note to the Qt readme about it.

The pro file is a bit outdated. To bring the pro file up to speed, we'd need to add the following missing .ui files under FORMS +=:

  • modeloverlay.ui
  • psbtoperationsdialog.ui

@hebasto
Copy link
Member Author

hebasto commented Apr 15, 2021

@jarolrod
Does Qt Creator allow to open *.ui file directly (as Qt Designer does)?

@hebasto
Copy link
Member Author

hebasto commented Apr 15, 2021

The pro file is a bit outdated. To bring the pro file up to speed, we'd need to add the following missing .ui files under FORMS +=:

* modeloverlay.ui

* psbtoperationsdialog.ui

... and remove aboutdialog.ui...

@jarolrod
Copy link
Contributor

jarolrod commented Apr 15, 2021

@hebasto yeah you can open up UI files individually. This pro file opens up all of them for you as well as the resources folder. I think it's more convenient to have all of them open. I don't have a strong opinion on this

It could be that no one uses it (i dont) and it could be wasting time to maintain this.

@hebasto
Copy link
Member Author

hebasto commented Apr 15, 2021

... it could be wasting time to maintain this.

That is my concern.

@Bosch-0
Copy link

Bosch-0 commented Apr 16, 2021

I personally do not use this as a designer. Qt Creator has its uses for working with Qt widgets to understand what can and can't be modified on the design side but outside that it's use as a design tool isn't great

ACK from me if this files requires resources to maintain

@jarolrod
Copy link
Contributor

jarolrod commented Apr 16, 2021

ACK 5f2be6e

ACK because I don't have a strong opinion, the file is not used very much, and the resulting conversation would be more man-hours than is used to maintain this file.

Qt Creator has its uses for working with Qt widgets to understand what can and can't be modified on the design side but outside that it's use as a design tool isn't great

There's a difference between designing and implementing a design by changing the UI files. Qt Creator, which packages Qt Designer, is the best tool for performing design work that changes the GUI as it's WYSIWYG.

we could add the two lines and remove the one line, that's the cost of maintenance for now. Or just remove the file. Again no strong opinion.

@fanquake fanquake changed the title Remove no longer used contrib/bitcoin-qt.pro from the repo contrib: remove no longer used contrib/bitcoin-qt.pro Apr 16, 2021
@fanquake fanquake merged commit 585cbe2 into bitcoin:master Apr 16, 2021
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Apr 16, 2021
@fanquake
Merge bitcoin#21695: Remove no longer used contrib/bitcoin-qt.pro fro…
3314807 
…m the repo
@hebasto hebasto deleted the 210415-pro branch April 16, 2021 07:48
@jarolrod jarolrod mentioned this pull request May 1, 2021
Closed
@bitcoin bitcoin 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

1 more reviewer

@jarolrod jarolrod jarolrod left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Scripts and tools

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@hebasto @laanwj @jarolrod @Bosch-0 @fanquake @DrahtBot