This makes it easier to launch Signet and testnet4 on Windows. Follows the same pattern as testnet.
Before:
After:
(the testnet4 icon is the same as testnet3, not in the above screenshot)
Add Signet and testnet4 launch shortcuts for Windows #26334
pull Sjors wants to merge 2 commits into bitcoin:master from Sjors:2022/10/windows-signet changing 3 files +8 −2-
Sjors commented at 3:05 pm on October 18, 2022: member
-
maflcko added the label GUI on Oct 18, 2022
-
maflcko added the label Build system on Oct 18, 2022
-
maflcko added the label DrahtBot Guix build requested on Oct 18, 2022
-
maflcko commented at 3:15 pm on October 18, 2022: memberDrahtBot may have build results in two days. Though, keep in mind that they are untrusted.
-
Sjors commented at 3:41 pm on October 18, 2022: memberI’m optimistic that I can install Guix on the new machine faster…
-
ghost commented at 4:00 pm on October 18, 2022: none
Concept ACK
Tested it on Windows and it works as expected except the color of logo for signet.
0 Directory: C:\Users\Test\AppData\Roaming\Microsoft\Windows\Start Menu\Programs\Bitcoin Core 1 2 3Mode LastWriteTime Length Name 4---- ------------- ------ ---- 5-a---- 18-10-2022 15:55 890 Bitcoin Core (64-bit).lnk 6-a---- 18-10-2022 15:55 1774 Bitcoin Core (signet, 64-bit).lnk 7-a---- 18-10-2022 15:55 1776 Bitcoin Core (testnet, 64-bit).lnk 8-a---- 18-10-2022 15:55 885 Uninstall Bitcoin Core (64-bit).lnk
-
Sjors commented at 4:16 pm on October 18, 2022: memberLooks like I need to set the correct yellow color.
-
jarolrod commented at 7:03 am on October 19, 2022: member
we have actual ico files under res for the main and testnet icons, you would need to introduce a bitcoin_signet.ico with the correct yellow hue applied under src/qt/res in order to introduce the correct icon.
Looking ahead, I’d like to move away from having these different launchers to being able to select the chain from within the gui.
-
-
DrahtBot commented at 2:36 pm on October 22, 2022: contributor
Guix builds
-
DrahtBot removed the label DrahtBot Guix build requested on Oct 22, 2022
-
luke-jr commented at 1:15 am on October 27, 2022: member
Most users won’t know what “signet” means, and it isn’t self-explanatory like “testnet”…
If you want a shortcut, please figure out a way to resolve this.
NACK as-is.
-
fanquake commented at 2:46 pm on February 6, 2023: memberWhat is the status of this? Seems very low priority, and not currently mergable.
-
DrahtBot commented at 2:46 pm on February 6, 2023: contributor
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
Code Coverage
For detailed information about the code coverage, see the test coverage report.
Reviews
See the guideline for information on the review process.
Type Reviewers ACK achow101 Concept NACK luke-jr, ghost Concept ACK TheCharlatan, michaelfolkson, laanwj, BrandonOdiwuor If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
Conflicts
No conflicts as of last run.
-
Sjors commented at 4:46 pm on February 6, 2023: memberOn my list to rebase, fix the icon color, and reduce potential confusion. I have a Windows setup now.
-
Sjors force-pushed on Feb 21, 2023
-
Sjors commented at 11:27 am on February 21, 2023: member
I added the word “test” to the application name.
Since Bitcoin Inquisition uses the default Signet to deploy several proposed soft forks, I think it’s useful to offer easier access (which requires the Inquisition folks to rebase after this is merged).
See #8285 for additional context.
I generated the icon from the mainnet icon by applying the same hue and saturation adjustments:
Though the -modulate command uses percentage instead of degrees, so it was a bit confusing to convert:
0convert src/qt/res/icons/bitcoin.ico -modulate 100,87,119.4 src/qt/res/icons/bitcoin_signet.icoI added a screenshot in the PR description.
The big icon looks correct, but the mini version in the search result does not. I probably need to change the ImageMagick command somehow. I could also try using the Testnet icon as the basis (and rotate the hue in the other direction).
-
Sjors force-pushed on Feb 21, 2023
-
Sjors marked this as ready for review on Feb 21, 2023
-
Sjors commented at 1:23 pm on February 21, 2023: member
I think I found the right incantation, using the high res PNG as input:
0convert src/qt/res/icons/bitcoin.png -modulate 100,87,119.4 -define icon:auto-resize="256,48,32,16" src/qt/res/icons/bitcoin_signet.icoI updated the PR description screenshot.
I noticed that testnet has 4 icon resolutions, which I’ve used for signet too. But mainnet has 9. I could regenerate that one, but it would only save 11kb.
-
Sjors force-pushed on Feb 21, 2023
-
in share/setup.nsi.in:145 in c80e55caee outdated
141@@ -141,6 +142,7 @@ Section -un.post UNSEC0001 142 Delete /REBOOTOK "$SMPROGRAMS\$StartMenuGroup\Uninstall $(^Name).lnk" 143 Delete /REBOOTOK "$SMPROGRAMS\$StartMenuGroup\$(^Name).lnk" 144 Delete /REBOOTOK "$SMPROGRAMS\$StartMenuGroup\@PACKAGE_NAME@ (testnet, 64-bit).lnk" 145+ Delete /REBOOTOK "$SMPROGRAMS\$StartMenuGroup\@PACKAGE_NAME@ (signet, 64-bit).lnk"
Sjors commented at 1:37 pm on February 21, 2023:Forgot to addtesthere, pushing fix.Sjors force-pushed on Feb 21, 2023ghost commented at 4:55 pm on February 21, 2023: noneMost users won’t know what “signet” means, and it isn’t self-explanatory like “testnet”…
Most users wont use it and it just helps devs and power users on windows. They know what signet means. Everyone else clicking on it is same as downloading a malware instead of bitcoin core which is not something this repo needs to solve. First match in search is always “bitcoin core (mainnet)” and even that would open other network if bitcoin.conf has other network set.
So this NACK makes no sense for windows users.
I added the word “test” to the application name.
Since Bitcoin Inquisition uses the default Signet to deploy several proposed soft forks, I think it’s useful to offer easier access (which requires the Inquisition folks to rebase after this is merged).
See #8285 for additional context.
I generated the icon from the mainnet icon by applying the same hue and saturation adjustments:
Though the -modulate command uses percentage instead of degrees, so it was a bit confusing to convert:
0convert src/qt/res/icons/bitcoin.ico -modulate 100,87,119.4 src/qt/res/icons/bitcoin_signet.icoI added a screenshot in the PR description.
The big icon looks correct, but the mini version in the search result does not. I probably need to change the ImageMagick command somehow. I could also try using the Testnet icon as the basis (and rotate the hue in the other direction).
NACK for “test signet”
signetalready helps to know its NOT mainnetSjors commented at 5:47 pm on February 21, 2023: memberI don’t see any harm in adding the word “test”. A user who searches “bitcoin” will see the “signet” entry in their search results and wonder what it is, perhaps even worry about it. Without the word “test” the only two ways to figure out are to google “what is signet” or to launch the application.
I don’t expect most users to know what “mainnet” is. They obviously know what Bitcoin is, but they may not know that one or more test networks exist.
ghost commented at 5:59 pm on February 21, 2023: noneI don’t expect most users to know what “mainnet” is. They obviously know what Bitcoin is, but they may not know that one or more test networks exist.
If someone doesnt know what is mainnet they wont use bitcoin core as a wallet.
TheCharlatan commented at 6:15 pm on April 25, 2023: contributorConcept ACKmichaelfolkson commented at 7:31 pm on April 25, 2023: contributorConcept ACK
Perhaps the Bitcoin Design community will have a view on the wording. I’d go with standalone “signet” but I haven’t done any user research on whether this isn’t sufficiently informative.
DrahtBot added the label CI failed on May 29, 2023DrahtBot removed the label CI failed on May 31, 2023in share/setup.nsi.in:145 in d4336ff18c outdated
141@@ -141,6 +142,7 @@ Section -un.post UNSEC0001 142 Delete /REBOOTOK "$SMPROGRAMS\$StartMenuGroup\Uninstall $(^Name).lnk" 143 Delete /REBOOTOK "$SMPROGRAMS\$StartMenuGroup\$(^Name).lnk" 144 Delete /REBOOTOK "$SMPROGRAMS\$StartMenuGroup\@PACKAGE_NAME@ (testnet, 64-bit).lnk" 145+ Delete /REBOOTOK "$SMPROGRAMS\$StartMenuGroup\@PACKAGE_NAME@ (test signet, 64-bit).lnk"
maflcko commented at 9:34 am on August 8, 2023:Can remove the64-bithere and above?
Sjors commented at 9:26 am on August 14, 2023:Done for testnet and signet. I didn’t touchName "@PACKAGE_NAME@ (64-bit)"though, not sure if that’s safe.in src/Makefile.qt.include:179 in d4336ff18c outdated
175@@ -176,6 +176,7 @@ QT_RES_ICONS = \ 176 qt/res/icons/address-book.png \ 177 qt/res/icons/bitcoin.ico \ 178 qt/res/icons/bitcoin_testnet.ico \ 179+ qt/res/icons/bitcoin_signet.ico \
maflcko commented at 9:36 am on August 8, 2023:Can just reuse the testnet icon to save 43.4kB in the git repo? Doesn’t seem worth it to burden every clone forever just so that a color in a test-only picture is of a slightly different tone.
Sjors commented at 9:19 am on August 14, 2023:Or we could just drop testnet3 :-P
I find the difference in color useful enough.
maflcko commented at 9:22 am on August 14, 2023:I find the difference in color useful enough.
Then, it would be better to have a tiny script in the build process to create the
.ico, no? This would make it reproducible, documented and easily modifiable, without on each modification bloating the git repo by 50kb
Sjors commented at 9:30 am on August 14, 2023:The incantation is in the commit message. Adding a script makes sense, but I’m not familiar enough with the (native) build tools to know where to add it. This particular command would also add a dependency to imagemagick (if we don’t use it already), though I’m sure the operation can be done with other tools if needed.
Seems better to do in a followup and generate the testnet icon that way too.
maflcko approvedmaflcko commented at 9:36 am on August 8, 2023: memberSeems fine. Left commentsSjors force-pushed on Aug 14, 2023DrahtBot added the label CI failed on Jan 13, 2024Sjors force-pushed on Feb 13, 2024maflcko commented at 1:02 pm on February 13, 2024: memberOn a second thought, I wonder what the target audience is. Someone aware of a test network, and wanting to use it, should be able to create a shortcut themselves? Creating the shortcut should be the easiest of their problems to get started with a test network.Sjors commented at 1:20 pm on February 13, 2024: memberCreating a shortcut is easy, but creating an icon with a non-confusing color is not.Sjors commented at 1:24 pm on February 13, 2024: memberI would also expect not-super-tech-savy people to be testing applications on custom signets, e.g. testing a vault GUI on Inquisition. I would hope they don’t use Windows of course :-)DrahtBot removed the label CI failed on Feb 13, 2024achow101 commented at 4:00 pm on April 9, 2024: memberThe PR didn’t seem to attract much attention in the past. Also, the issue seems not important enough right now to keep it sitting around idle in the list of open PRs.
Closing due to lack of interest.
achow101 closed this on Apr 9, 2024
achow101 reopened this on Apr 9, 2024
laanwj requested review from laanwj on Apr 14, 2024laanwj commented at 9:28 am on April 14, 2024: memberConcept ACK, i think it’s important that signet gets its own launch icon if testnet has, it’s more useful for testing than testnet. The other option would be to keep only the mainnet icon. But i think there’s value in making windows users aware of these kind of things (so they don’t reinvent wheels, or, test on mainnet 😅 ).BrandonOdiwuor commented at 4:18 pm on April 15, 2024: contributorConcept ACKDrahtBot added the label Needs rebase on Sep 2, 202477b2923f87Add Signet launch shortcut for Windows
cd src/qt/res/icons convert bitcoin.png -modulate 100,87,119.4 -define icon:auto-resize="256,48,32,16" bitcoin_signet.ico This commit also removes the 64-bit mention from testnet.
Add Testnet4 launch shortcut for Windows cfd03de965Sjors force-pushed on Sep 3, 2024Sjors commented at 8:34 am on September 3, 2024: memberRebased for CMake (droppingsrc/Makefile.qt.include) and added a testnet4 shortcut.Sjors renamed this:
Add Signet launch shortcut for Windows
Add Signet and testnet4 launch shortcuts for Windows
on Sep 3, 2024DrahtBot removed the label Needs rebase on Sep 3, 2024achow101 commented at 10:43 pm on September 3, 2024: memberACK cfd03de965a081facbd72316c76603dd7aa511bd
Did not test, but the code matches the pattern used for the other networks.
DrahtBot requested review from TheCharlatan on Sep 3, 2024DrahtBot requested review from BrandonOdiwuor on Sep 3, 2024
github-metadata-mirror
This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-09-28 22:12 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me
