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.

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

docs: Update macdeploy README to include all files produced by make deploy #17142

Merged
merged 1 commit into from
Oct 15, 2019
Merged

Conversation

za-kk
Copy link
Contributor

@za-kk za-kk commented Oct 14, 2019

Fixes issue #16909 to update the contrib/macdeploy/README.md to match the files produced from make deploy

The files produced from make deploy are as follows:

  • Bitcoin-Qt.dmg
  • Bitcoin Core.app
  • dist/Bitcoin Core.app

@fanquake fanquake added the Docs label Oct 14, 2019
@fanquake fanquake changed the title Update contrib/macdeploy/README.md to match files produced from make deploy docs: Update macdeploy README to include all files produced by make deploy Oct 14, 2019
Copy link
Member

@fanquake fanquake left a comment

Choose a reason for hiding this comment

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

Thanks. Can you please update your commit message to something like the PR title.

contrib/macdeploy/README.md Outdated Show resolved Hide resolved
@@ -11,5 +11,5 @@ This script should not be run manually, instead, after building as usual:
During the process, the disk image window will pop up briefly where the fancy
settings are applied. This is normal, please do not interfere.

When finished, it will produce `Bitcoin-Core.dmg`.
When finished, it will produce `Bitcoin-Qt.dmg`, `Bitcoin Core.app` and `dist/Bitcoin Core.app`.
Copy link
Member

Choose a reason for hiding this comment

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

Two .apps? Is this intentional?

Copy link
Member

@laanwj laanwj Oct 15, 2019

Choose a reason for hiding this comment

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

I think it'd make sense to make this a list, and describe what each item is (and what the differences are)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The two .app files were intentional as both are created, one in the root folder and the other is in the dist.

I am unsure of the differences between these two .app files, so any insight into this would be appreciated

Copy link
Member

@laanwj laanwj Oct 15, 2019

Choose a reason for hiding this comment

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

OK—I think there's a misunderstanding about the purpose of this documentation. The idea here is not to list every intermediate that will be included in the image, but only the end product of the macdeploy (the .dmg image). If the purpose of the intermediate files is not clear, better to not list them.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay thanks @laanwj, I will reverse the change back to just listing the .dmg file as suggested and keep the filename change to match what is actually produced from make deploy (Bitcoin-Qt.dmg)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done and squashed the commits 👍

@jonasschnelli
Copy link
Contributor

ACK 4441e58

@fanquake
Copy link
Member

ACK 4441e58 - checked that Bitcoin-Qt.dmg is produced.

fanquake added a commit that referenced this pull request Oct 15, 2019
…uced by `make deploy`

4441e58 Update macdeploy README to include correctly named `.dmg` file produced from `make deploy` (Zakk)

Pull request description:

  Fixes issue #16909 to update the `contrib/macdeploy/README.md` to match the files produced from `make deploy`

  The files produced from `make deploy` are as follows:

  - `Bitcoin-QT.dmg`
  - `Bitcoin Core.app`
  - `dist/Bitcoin Core.app`

ACKs for top commit:
  jonasschnelli:
    ACK 4441e58
  fanquake:
    ACK 4441e58 - checked that `Bitcoin-Qt.dmg` is produced.

Tree-SHA512: 99bfadab59c7c516005b051e4a369f330178313a284bb665c22c40f70a6159f175909c08db1b32976ad7b130b53b414f8ba96f8ff7cbd164f2724c0cc151704a
@fanquake fanquake merged commit 4441e58 into bitcoin:master Oct 15, 2019
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Dec 16, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants