new splash screen #2495

pull jonasschnelli wants to merge 5 commits into bitcoin:master from jonasschnelli:new_splash_screen changing 9 files +103 −13
  1. jonasschnelli commented at 1:26 PM on April 9, 2013: contributor

    why:

    • the current splash-screen has no referring to official images on https://en.bitcoin.it/wiki/Promotional_graphics
    • the current splash screen only exists in a low res jpg
    • current splash screen looks dark and "hackish"
    • new splash screen should generate positive, "trust-emotions".
    • new splash screen gives the user infos about the running client.
    • new splash screen can handle long messages (in a lot of languages the text is cropped in current release)
    • example: https://dl.dropbox.com/u/7383846/new_bitcoin_splash.png
    • new size (x2) 400x312
    • contains textual information about the client
    • textinfos are dynamicly written to the image
    • when -testnet is switch on, the splashscreen will show the bitcoin logo in testnet-color (as well as a text [testnet])
  2. jonasschnelli commented at 1:27 PM on April 9, 2013: contributor

    If the new splash screen will be accepted, i will update the build process so that the splash.png text (version, copyright) gets automatically "written" with imagemagick like the gui about screen.

  3. laanwj commented at 2:13 PM on April 9, 2013: member

    I like the splash screen. However we should add the version and copyright information text programmatically from qt. It does not need to be in the image. Otherwise there's yet another place to update copyright and version numbers.

  4. in src/qt/bitcoin.qrc:None in 465a85604b outdated 
       0 | @@ -1,4 +1,4 @@
   1 | -<!DOCTYPE RCC><RCC version="1.0">
    Diapolo commented at 3:00 PM on April 9, 2013:

    Please don't remove that tag.

    jonasschnelli commented at 7:02 PM on April 9, 2013:

    Oops. was auto-removed by Qt Creator. Now readded in d33ff50.

  5. Diapolo commented at 3:02 PM on April 9, 2013: none

    @laanwj ACK, I thought the same. We don't want to update that screen with every new year or relase. @jonasschnelli Looks very nice, I like it.

  6. jonasschnelli commented at 7:06 PM on April 9, 2013: contributor

    @laanwj @Diapolo as a coder, i would also recommend the option of placing the text on the splashscreen by code. As a designer i would avoid this. Why: text placed by Qt will look much more sharp and somehow crispy. Text placed as image on a template image by imagemagick (or other command-line capable gfx tool) will look much better and can use non-standard fonts.

    It might sounds crazy for you (coders), but in my eyes, the splash screen is the first contact with the enduser and when it come to the point where the Bitcoin-Qt client gets "mainstream", first contact is very important. That's why i would go with the pre-generated png in the build process with imagemagick.

    It's more work for us, but more quality for the enduser. And i kind of like this.

  7. laanwj commented at 7:47 PM on April 9, 2013: member

    Qt supports various text rendering options as well. And TBH it's our time that is very limited at the moment, I really don't mind text quality to be somewhat less for that.

  8. laanwj commented at 7:50 PM on April 9, 2013: member

    Maybe there's another option: make building the image part of Bitcoin-Qt's build process. That makes it automatic, which is fine with me as well. Non-standard fonts be a problem in any case though: everyone needs to be able to build the image, so it can't rely on certain fonts to be installed.

  9. jonasschnelli commented at 7:56 PM on April 9, 2013: contributor

    @laanwj yes. Include into bitcoin-qt's build process. Font: i would just place a ttf or otf file (open source fonts) into the qt/src folder. The font must not be installed on the build-system.

  10. laanwj commented at 8:04 PM on April 9, 2013: member

    @diapolo would calling imagemagick in the build process work on windows? I suppose it'd be more difficult...

  11. jonasschnelli commented at 6:39 AM on April 10, 2013: contributor

    @laanwj i thought the same. Imagemagick is probably a overkill. Has also huge dependencies (Ghostscript, freetype). And yes: windows user would hate me. ;)

    I think I try to create a solution with qt only (runtime). Let me try to play with http://qt-project.org/doc/qt-5.0/qtgui/qrawfont.html#alphaMapForGlyph.

    Will push something soon.

  12. gmaxwell commented at 6:43 AM on April 10, 2013: contributor

    meh. I love the image, but it seems like such a small thing to need to invoke imagemagick for... I'm sure that QT can be made to do this.

  13. laanwj commented at 7:13 AM on April 10, 2013: member

    Agreed @ gmaxwell. Especially as Qt has a quite advanced rendering backend (QPainter). I daresay it can do most that imagemagick can do, rendering-wise.

    Let's do this the proper way :)

  14. Diapolo commented at 10:50 AM on April 10, 2013: none

    @laanwj I've never heard of imagemagick, but as we want to do it the Qt way I won't even google it ^^.

  15. jonasschnelli commented at 2:00 PM on April 10, 2013: contributor

    Just pushed some code which places the infos by Qt. In development, the version string is much longer and a smaller font will be used. Releases would look like this (1:1 screen from the splash screen drawing by qt): Bildschirmfoto 2013-04-10 um 15 52 38

    Question:

    • Where should we place the 2013 (© end year)?
    • Is there a nice way to get the app name dynamically?

    Could someone test this on Qt4.7/4.8?

  16. Diapolo commented at 2:07 PM on April 10, 2013: none

    @jonasschnelli You could use QApplication::applicationName() for the name, but be careful, as it would be Bitcoin-Qt-testnet for testnet currently.

  17. in src/qt/bitcoin.cpp:None in f77d3fc0e0 outdated 
     201 | +    int titleCopyrightVSpace    = 40;
 202 | +
 203 | +    // define text to place
 204 | +    QString titleText       = QString("Bitcoin-Qt");
 205 | +    QString versionText     = QString("Version %1").arg(QString::fromStdString(FormatFullVersion())); //!<set the version string without client model (it's not available yet)
 206 | +    QString copyrightText   = QString("© 2009-%1\nThe Bitcoin developers").arg(2013); //TODO: where we gonna place the copyright endyear (also check aboutdialog.cpp!)?
    Diapolo commented at 2:14 PM on April 10, 2013:

    Currently we have ABOUTDIALOG_COPYRIGHT_YEAR in aboutdialog.cpp, which perhaps could be moved to another location for this to be usable.

    jonasschnelli commented at 2:16 PM on April 10, 2013:

    Yes. I saw it. Is there a nice location for this const?

    Diapolo commented at 2:21 PM on April 10, 2013:

    Perhaps it could be added to clientversion.h, but I'm not sure if we should use that directly in bitcoin.cpp. @laanwj What do you think? If we would include it, you also had options to generate the version string without the client model (which seems bad, because we HAVE it in clientmodel to not access that stuff directly AFAIK).

    Edit: Is it possible to use tr("Copyright") + QString(" &copy; ") + tr("2009-%1 The Bitcoin developers").arg(2013), because we should allow translation and had problems with that copyright sign, if we don't use the HTML tag for it :).

    laanwj commented at 2:39 PM on April 10, 2013:

    Agreed. Clientversion.h sounds like the right place, then all the version and year stuff can be updated in the same place.

    laanwj commented at 2:42 PM on April 10, 2013:

    @jonas won't including two large splash images, grow the executable quite a lot? If so, it's not really worth it for the testnet imo; and putting "testnet" in the text is enough.

  18. jonasschnelli commented at 2:29 PM on April 10, 2013: contributor
    • App name is now dynamic ("-testnet" will be replaced with " (tn)" for a better style).
    • When on testnet, the splash screen contains also a "green" bitcoin logo.
  19. Diapolo commented at 2:42 PM on April 10, 2013: none

    @jonasschnelli We currently use [testnet] appended to Bitcoin-Qt to indicate testnet usage (see bitcoingui.cpp -> setWindowTitle(windowTitle() + QString(" ") + tr("[testnet]"));), which I would like to keep (with equality and translation stuff in my mind).

  20. in src/qt/bitcoin.qrc:None in cb7ed0eb63 outdated 
      42 | @@ -43,7 +43,8 @@
  43 |      </qresource>
  44 |      <qresource prefix="/images">
  45 |          <file alias="about">res/images/about.png</file>
  46 | -        <file alias="splash">res/images/splash2.jpg</file>
  47 | +        <file alias="splash">res/images/splash.png</file>
  48 | +        <file alias="splash_testnet">res/images/splash_testnet.png</file>
    Diapolo commented at 2:43 PM on April 10, 2013:

    You also once more will need to mention them in the assets file :D.

    jonasschnelli commented at 2:45 PM on April 10, 2013:

    Oh.. yeah. Comes also.

  21. in src/qt/bitcoin.cpp:None in cb7ed0eb63 outdated 
     199 | +    int paddingTop              = 50;
 200 | +    int titleVersionVSpace      = 17;
 201 | +    int titleCopyrightVSpace    = 40;
 202 | +
 203 | +    // define text to place
 204 | +    QString titleText       = QString(QApplication::applicationName()).replace(QString("-testnet"), QString(" (tn)"), Qt::CaseSensitive);
    Diapolo commented at 2:44 PM on April 10, 2013:

    replace(QString("-testnet"), QString(" ") + tr("[testnet]")), if that compiles :).

    jonasschnelli commented at 2:45 PM on April 10, 2013:

    The problem is more about the length of the text. But i can also adjust the font size. Let me do it... :)

    Diapolo commented at 2:47 PM on April 10, 2013:

    I can be quite hard to satisfy, sorry ^^. But it makes sense to use translations and the best is to use the ones we already HAVE :).

    jonasschnelli commented at 2:50 PM on April 10, 2013:

    i just saw that "tr function" is not available at that stage. But i did not investigate that deep.

    Diapolo commented at 2:57 PM on April 10, 2013:

    Is our _() fuction working there, see ui_interface.h, which allows translations before the UI is setup.

  22. in src/qt/bitcoin.cpp:None in cb7ed0eb63 outdated 
     201 | +    int titleCopyrightVSpace    = 40;
 202 | +
 203 | +    // define text to place
 204 | +    QString titleText       = QString(QApplication::applicationName()).replace(QString("-testnet"), QString(" (tn)"), Qt::CaseSensitive);
 205 | +    QString versionText     = QString("Version %1").arg(QString::fromStdString(FormatFullVersion())); //!<set the version string without client model (it's not available yet)
 206 | +    QString copyrightText   = QString("© 2009-%1\nThe Bitcoin developers").arg(2013); //TODO: where we gonna place the copyright endyear (also check aboutdialog.cpp!)?
    Diapolo commented at 2:46 PM on April 10, 2013:

    Your rebase removed my comment here, try to use tr("Copyright") + QString(" &copy; ") + tr("2009-%1 The Bitcoin developers").arg(2013) (see aboutdialog.cpp) and we still need @laanwj to comment about the ABOUTDIALOG_COPYRIGHT_YEAR constant.

  23. jonasschnelli commented at 2:52 PM on April 10, 2013: contributor

    @laanwj the testnet splash png is 45.37kb. When you compare it against the blockchain size... but yes: it will increase the bin size. I still recommend to have it (the new testnet splash).

  24. Diapolo commented at 2:58 PM on April 10, 2013: none

    I also like the new testnet splash :) and 46KB is fine with me!

  25. sipa commented at 3:01 PM on April 10, 2013: member

    I'm fine with 46kB extra.

  26. laanwj commented at 3:33 PM on April 10, 2013: member

    46k is much less than I had estimated, I'm fine with that.

  27. qubez commented at 5:51 PM on April 10, 2013: none

    splash screen in <3k: splash-3k splash-3k-testnet

    (I <3 Bitcoin)

  28. Diapolo commented at 6:46 AM on April 11, 2013: none

    @jonasschnelli Can you please squash all commits into one after we have the final ACK for this :).

  29. in src/qt/bitcoin.cpp:None in e628434ef4 outdated 
     199 | +    int paddingTop              = 50;
 200 | +    int titleVersionVSpace      = 17;
 201 | +    int titleCopyrightVSpace    = 40;
 202 | +
 203 | +    // define text to place
 204 | +    QString titleText       = QString(QApplication::applicationName()).replace(QString("-testnet"), QString(" [testnet]"), Qt::CaseSensitive);
    Diapolo commented at 7:05 AM on April 11, 2013:

    Can you try if QString(" ") + _("[testnet]") instead of QString(" [testnet]") works please. We have [testnet] already translated [testnet] is a NEW string and for that reason currently untranslated.

    jonasschnelli commented at 11:40 AM on April 11, 2013:

    For sure. " [testnet]" is a bad label. Will change that.

  30. in src/qt/bitcoin.cpp:None in e628434ef4 outdated 
     201 | +    int titleCopyrightVSpace    = 40;
 202 | +
 203 | +    // define text to place
 204 | +    QString titleText       = QString(QApplication::applicationName()).replace(QString("-testnet"), QString(" [testnet]"), Qt::CaseSensitive);
 205 | +    QString versionText     = QString("Version %1").arg(QString::fromStdString(FormatFullVersion())); //!<set the version string without client model (it's not available yet)
 206 | +    QString copyrightText   = QString("© 2009-%1\n")+QString(QObject::tr("The Bitcoin developers")).arg(COPYRIGHT_YEAR); //TODO: where we gonna place the copyright endyear (also check aboutdialog.cpp!)?
    Diapolo commented at 7:13 AM on April 11, 2013:

    Does this work? Should be cleaner and as I said uses our internal translations function _() + HTML instead of ©. QString("&copy; 2009-%1<br>").arg(COPYRIGHT_YEAR) + _("The Bitcoin developers");

    jonasschnelli commented at 11:41 AM on April 11, 2013:

    &copy; does not work because we placing text on a QPixmap. Ist must be the raw char. Do you think we have problem with special chars in the code? As i see everything is UTF-8.

    Diapolo commented at 11:53 AM on April 11, 2013:

    We had that problem on the aboutdialog page, that is why I was asking :). But even if HTML doesn't work, you should still try to use QString("© 2009-%1\n").arg(COPYRIGHT_YEAR) + _("The Bitcoin developers");

    laanwj commented at 11:58 AM on April 11, 2013:

    Instead of unicode in the source code use QChar(...) please. Pre-C++11, source code must be ASCII not UTF-8.

    jonasschnelli commented at 11:58 AM on April 11, 2013:

    _() function does not work here. Compiler claims about casting. Maybe a std::string:c_string could help... but what speaks against QObject::tr()?

    Diapolo commented at 12:01 PM on April 11, 2013:

    AFAIK we nowhere else use that QObject::tr(), but I think @laanwj can comment on that :). Edit: QString::fromStdString(_("The Bitcoin developers")); seems to work here.

    QString("© 2009-%1\n").arg(COPYRIGHT_YEAR) + QString::fromStdString(_("The Bitcoin developers"));

    laanwj commented at 12:16 PM on April 11, 2013:

    You can only use the _ function in classes derived from QObject. This is used to fill in the context. ::tr is more general and takes an explicit context identifier. Only use it when you really need to (such as here in the main scope. Another option may be to create a class for the splash screen)

    laanwj commented at 12:30 PM on April 11, 2013:

    Eh I meant the tr function withput QObject qualifier, not _. _ must not be used in qt code at all, it's a hack to get messages in the core translated where we can't use qt.

  31. in src/qt/bitcoin.cpp:None in e628434ef4 outdated 
     215 | +    }
 216 | +
 217 | +    QPainter pixPaint(&pixmap);
 218 | +    pixPaint.setPen(QColor(100,100,100));
 219 | +
 220 | +    pixPaint.setFont(QFont("Helvetica", 33));
    Diapolo commented at 7:16 AM on April 11, 2013:

    Just asking, is this some default font the client is using or did you pick it?

    jonasschnelli commented at 7:25 AM on April 11, 2013:

    It's a designers choice. ;) All OS should be fine with it. If not, Arial will be taken which is only slightly different to Helvetia. Okay?

    Diapolo commented at 7:30 AM on April 11, 2013:

    Jup :).

  32. new splash screen
    why:
- the current splash-screen has no referring to official images on https://en.bitcoin.it/wiki/Promotional_graphics
- the current splash screen only exists in a low res jpg
- current splash screen looks dark and "hackish"
- new splash screen should generate positive, "trust-emotions".
- new splash screen gives the user infos about the running client.
- new splash screen can handle long messages (in a lot of languages the text is cropped in current release)

- example: https://dl.dropbox.com/u/7383846/new_bitcoin_splash.png
- new size (x2) 400x312
- contains textual information about the client
- textinfos are dynamicly written to the image
- when -testnet is switch on, the splashscreen will show the bitcoin logo in testnet-color (as well as a text [testnet])

Signed-off-by: Jonas Schnelli <jonas.schnelli@include7.ch>
    2497ef513b
  33. jonasschnelli commented at 12:56 PM on April 11, 2013: contributor

    squashed and ready to test on Qt4.8. Anyone?

  34. jonasschnelli commented at 12:56 PM on April 11, 2013: contributor

    Pulltester might also do a check...

  35. Merge branch 'master' of git://github.com/bitcoin/bitcoin into new_splash_screen
    Signed-off-by: Jonas Schnelli <jonas.schnelli@include7.ch>

Conflicts:
	doc/assets-attribution.txt
    21d096357c
  36. fanquake commented at 1:27 PM on April 11, 2013: member

    @jonasschnelli I can test for you. Have one comp running 10.7.5

  37. jonasschnelli commented at 1:28 PM on April 11, 2013: contributor

    @fanquake do you can build from the source (take master and pull from jonasschnelli/bitcoin new_splash_screen)?

  38. copyright char as proper QChar
    Signed-off-by: Jonas Schnelli <jonas.schnelli@include7.ch>
    66fa5cbaef
  39. in src/qt/bitcoin.cpp:None in 21d096357c outdated 
     201 | +    int titleCopyrightVSpace    = 40;
 202 | +
 203 | +    // define text to place
 204 | +    QString titleText       = QString(QApplication::applicationName()).replace(QString("-testnet"), QString(""), Qt::CaseSensitive);
 205 | +    QString versionText     = QString("Version %1").arg(QString::fromStdString(FormatFullVersion())); //!<set the version string without client model (it's not available yet)
 206 | +    QString copyrightText   = QString("\u00A9 2009-%1\n").arg(COPYRIGHT_YEAR) + QString(BitcoinGUI::tr("The Bitcoin developers")); //TODO: where we gonna place the copyright endyear (also check aboutdialog.cpp!)?
    laanwj commented at 1:52 PM on April 11, 2013:

    I don't think \uXXXX will work in non-gcc compilers such as MSVC. Better use QChar(0xA9) + QString(" ...") .

    jonasschnelli commented at 2:15 PM on April 11, 2013:

    sorry for my lack of non-gcc experience. Fixed now.

  40. BitcoinPullTester commented at 3:02 PM on April 11, 2013: none

    Automatic sanity-testing: FAILED BUILD/TEST, see http://jenkins.bluematt.me/pull-tester/66fa5cbaefc7bbf0a5ac48303d72818088c9f04e for binaries and test log.

    This could happen for one of several reasons:

    1. It chanages paths in makefile.linux-mingw or otherwise changes build scripts in a way that made them incompatible with the automated testing scripts (you can find the patches applied at test-time at http://jenkins.bluematt.me/pull-tester/files/patches/)
    2. It adds/modifies tests which test network rules (thanks for doing that), which conflicts with a patch applied at test time
    3. It does not build on either Linux i386 or Win32 (via MinGW cross compile)
    4. The test suite fails on either Linux i386 or Win32
    5. The block test-cases failed (lookup the first bNN identifier which failed in https://github.com/TheBlueMatt/test-scripts/blob/master/FullBlockTestGenerator.java)

    If you believe this to be in error, please ping BlueMatt on freenode or TheBlueMatt here.

    This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.

  41. jonasschnelli commented at 3:06 PM on April 11, 2013: contributor

    Oh... Hmm.. "src/qt/bitcoin.cpp:205: error: 'COPYRIGHT_YEAR' was not declared in this scope" Wasn't it merged in a PR to the master in #2503?

  42. Diapolo commented at 3:12 PM on April 11, 2013: none

    You can rebase to current master and update this pull, perhaps @BitcoinPullTester was doing it's work with a not up-to-date version :).

  43. in doc/assets-attribution.txt:None in 66fa5cbaef outdated 
      51 | @@ -57,6 +52,7 @@ License: Oxygen icon theme is dual licensed. You may copy it under the Creative
  52 |  Icon: src/qt/res/icons/bitcoin.icns, src/qt/res/src/bitcoin.svg,
  53 |        src/qt/res/src/bitcoin.ico, src/qt/res/src/bitcoin.png,
  54 |        src/qt/res/src/bitcoin_testnet.png, docs/bitcoin_logo_doxygen.png,
  55 | -      src/qt/res/icons/toolbar.png, src/qt/res/icons/toolbar_testnet.png
  56 | +      src/qt/res/images/splash.png, src/qt/res/images/splash_testnet.png,
  57 | +	  src/qt/res/icons/toolbar.png
    Diapolo commented at 3:13 PM on April 11, 2013:

    Looks like an indention via tab ;).

  44. in src/qt/bitcoin.cpp:None in 66fa5cbaef outdated 
     199 | +    int paddingTop              = 50;
 200 | +    int titleVersionVSpace      = 17;
 201 | +    int titleCopyrightVSpace    = 40;
 202 | +
 203 | +    // define text to place
 204 | +    QString titleText       = QString(QApplication::applicationName()).replace(QString("-testnet"), QString(""), Qt::CaseSensitive);
    Diapolo commented at 3:14 PM on April 11, 2013:

    Now you are leaving out the [testnet] completely in the splash screen, right?

    jonasschnelli commented at 8:43 AM on April 12, 2013:

    the testnet will be cut of the string, but further down you will see a part of code who places a "[testnet]" string if testnet is on in the upper right corner. Looks good in my eye.

  45. in src/qt/bitcoin.cpp:None in 66fa5cbaef outdated 
     201 | +    int titleCopyrightVSpace    = 40;
 202 | +
 203 | +    // define text to place
 204 | +    QString titleText       = QString(QApplication::applicationName()).replace(QString("-testnet"), QString(""), Qt::CaseSensitive);
 205 | +    QString versionText     = QString("Version %1").arg(QString::fromStdString(FormatFullVersion())); //!<set the version string without client model (it's not available yet)
 206 | +    QString copyrightText   = QChar(0xA9)+QString(" 2009-%1\n").arg(COPYRIGHT_YEAR) + QString(BitcoinGUI::tr("The Bitcoin developers")); //TODO: where we gonna place the copyright endyear (also check aboutdialog.cpp!)?
    Diapolo commented at 3:16 PM on April 11, 2013:

    Your comment can be removed and if you want to make me happy :-P use a space before and after the first +.

    jonasschnelli commented at 8:44 AM on April 12, 2013:

    Okay. Will fix that.

  46. in src/qt/bitcoin.cpp:None in 66fa5cbaef outdated 
     200 | +    int titleVersionVSpace      = 17;
 201 | +    int titleCopyrightVSpace    = 40;
 202 | +
 203 | +    // define text to place
 204 | +    QString titleText       = QString(QApplication::applicationName()).replace(QString("-testnet"), QString(""), Qt::CaseSensitive);
 205 | +    QString versionText     = QString("Version %1").arg(QString::fromStdString(FormatFullVersion())); //!<set the version string without client model (it's not available yet)
    Diapolo commented at 3:18 PM on April 11, 2013:

    That comment is also obsolete, but AFAIK we need to include clientversion.h now below #include "paymentserver.h" in this files header.

    jonasschnelli commented at 8:44 AM on April 12, 2013:

    yeah. I'll fix that...

  47. sipa commented at 5:32 PM on April 11, 2013: member

    I like the splash screen image :)

  48. Diapolo commented at 5:53 PM on April 11, 2013: none

    Same here, I really love it, great looking! Don't think my many comments lower that feeling :) @jonasschnelli .

  49. in src/qt/bitcoin.cpp:None in 66fa5cbaef outdated 
     192 | @@ -192,7 +193,64 @@ int main(int argc, char *argv[])
 193 |          return 1;
 194 |      }
 195 |  
 196 | -    QSplashScreen splash(QPixmap(":/images/splash"), 0);
 197 | +    // set reference point, paddings
    laanwj commented at 3:44 AM on April 12, 2013:

    Code-wise, I would prefer if you move this stuff to a function or class. The main is already pretty cluttered. Personally I prefer a main function that consists of function/method calls, not logic and rendering code in itself.

    jonasschnelli commented at 8:46 AM on April 12, 2013:

    Let me inherit the QSplashScreen and write it more proper to reduce bitcoin.cpp size.

  50. jonasschnelli commented at 8:45 AM on April 12, 2013: contributor

    @Diapolo my contributions are for the blockchain and not for my ego. :) so keep on finding details to make it better!

  51. sipa commented at 8:58 PM on April 12, 2013: member

    Seems to work fine. I haven't checked the code changes, but can you at least squash them?

  52. jonasschnelli commented at 11:56 AM on April 13, 2013: contributor

    @sipa: will finish the splash screen soon (1-2 days) then i try to squash. I once pulled/updates from master, ... i think i can't squash over the merge of the master? Can i?

  53. laanwj commented at 12:37 PM on April 13, 2013: member

    @jonasschnelli it is possible, but more difficult, and not simply with git rebase -i (I think. That's why you should ideally not merge in these cases, but always rebase); easiest may be to start from a new branch with master, then git cherry-pick the non-merge commits.

  54. Merge branch 'master' of git://github.com/bitcoin/bitcoin into new_splash_screen ab08de4895
  55. own class for splashscreen e7467cce26
  56. jonasschnelli closed this on Apr 14, 2013
  57. jonasschnelli commented at 9:44 AM on April 14, 2013: contributor

    fresh and clean pull request is #2524

  58. sipa commented at 9:59 AM on April 14, 2013: member

    You can just do a force push to the branch associated with the old pullrequest. You don't have to create a new one.

  59. jonasschnelli commented at 11:54 AM on April 14, 2013: contributor

    @sipa: i did a --force push but the "old commits" where still in the commit list. The branch on my github repo was completely different to what i had localy (and pushed with --force). That's why i then restarted the whole thing. Hope you excuse. :)

  60. laanwj commented at 2:35 PM on April 14, 2013: member

    Yes it's not a problem, he's just saying that in general you never need to re-create a pull request. Force pushing something new will override anything that is currently in the pull request. Sometimes, the old commits stay there for a while ~ 5 minutes but that's just github in the process of updating.

  61. DrahtBot locked this on Sep 8, 2021
Contributors
jonasschnellilaanwjDiapologmaxwellsipaqubezfanquakeBitcoinPullTester
Linked (view graph)
#2503 new COPYRIGHT_YEAR macro in clientversion.h
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: 2026-04-21 21:16 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me