Skip to content
This repository has been archived by the owner on Sep 5, 2020. It is now read-only.

change binary name "Ethereum-Wallet" to "Ethereum Wallet" #883

Closed
wants to merge 1 commit into from

Conversation

luclu
Copy link
Contributor

@luclu luclu commented Jun 24, 2016

0.7.5 used Ethereum Wallet while
0.7.6 used Ethereum-Wallet again
as reported in #881 (OSX app consistent naming)

This PR closes #833.

Tested on OSX 10.11.5, Ubuntu 14.04 and Windows 7 x64.

@@ -113,7 +113,7 @@ gulp.task('set-variables-mist', function () {
gulp.task('set-variables-wallet', function () {
type = 'wallet';
filenameLowercase = 'ethereum-wallet';
Copy link
Contributor Author

@luclu luclu Jun 24, 2016

Choose a reason for hiding this comment

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

btw.. what is this?

Copy link
Member

Choose a reason for hiding this comment

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

idk... it isn't used across the entire project.

Copy link
Contributor

Choose a reason for hiding this comment

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

That code is used as part of the build process.

Copy link
Contributor

Choose a reason for hiding this comment

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

This was used in an earlier stage of the build script. we can probably get rid of it now

@alexvandesande
Copy link

Previously I was changing the filenames manually. This change you made also would impact on the folder structure and windows platform. Not that simple

@hiddentao
Copy link
Contributor

Seems to me that it's just the Mac app folder name which needs to be set. Am I right?

@frozeman
Copy link
Contributor

frozeman commented Jun 28, 2016

Changing the file name will most likely have effects on other places. @luclu did you build the files and see if it all works?

@luclu
Copy link
Contributor Author

luclu commented Jun 28, 2016

I build and diffed all the files and folders.
The only changes happen to the binary name and the platform's folder name.
Both get set during electron-packager's name variable gulpfile.js#L301.

@frozeman
Copy link
Contributor

can you remove the filenameLowercase then

@luclu
Copy link
Contributor Author

luclu commented Jun 28, 2016

I will try to find another variable of electron-packager to differentiate the folder from the binary name. Otherwise we might have to leverage gulp to rename it afterwards.

@frozeman
Copy link
Contributor

Yeah the folder name and possible problems with some OS, or servers is why i kept the dash.
I'm not sure if introducing a lot of complicity makes sense here.

If we go that route than rather renaming the file names afterwards. The problem is renaming in gulp is not straight forward, as you need to copy, rename and delete the old one. I do this quite a lot in the current gulp script already.

So i vote for less complexity.

@luclu
Copy link
Contributor Author

luclu commented Jun 28, 2016

I will try to leverage https://www.npmjs.com/package/gulp-rename - haha funny nodejs world 60.000 download in the last day..

@frozeman
Copy link
Contributor

we already use gulp-rename, but like i said it doesn't remove the old version, so you need to have a second step removing it yourself. It just adds complexity imo

@luclu
Copy link
Contributor Author

luclu commented Jul 25, 2016

On hold until #972 is merged.

@luclu
Copy link
Contributor Author

luclu commented Sep 17, 2016

Done in PR #1159 (former #972).

@luclu luclu closed this Sep 17, 2016
@luclu luclu deleted the change_wallet_name branch September 30, 2016 10:13
@lock
Copy link

lock bot commented Mar 31, 2018

This thread has been automatically locked because it has not had recent activity. Please open a new issue for related bugs and link to relevant comments in this thread.

@lock lock bot locked and limited conversation to collaborators Mar 31, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

rename "Ethereum-Wallet" to "Ethereum Wallet"
5 participants