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

Rename remaining Atom Shell references #5683

Merged
merged 9 commits into from May 26, 2016

Conversation

Projects
None yet
4 participants
@kevinsawicki
Contributor

kevinsawicki commented May 24, 2016

This pull request removes all references to the old Atom Shell name from the repo.

After seeing #5670 it seems worth changing the remaining helper functions and environment variables to say Electron instead of Atom Shell to prevent confusion.

/cc @electron/maintainers

@mnquintana

This comment has been minimized.

Member

mnquintana commented May 24, 2016

What do you think about also renaming all the file / directory / class / namespace names in the repo that still include atom?

@kevinsawicki

This comment has been minimized.

Contributor

kevinsawicki commented May 24, 2016

What do you think about also renaming all the file / directory / class / namespace names in the repo that still include atom?

I think that should be done as well eventually in probably a few different pull requests since that is much more extensive.

@paulcbetts

This comment has been minimized.

Contributor

paulcbetts commented May 24, 2016

@kevinsawicki This is highly likely to break people's CI setups (including our own) - maybe support both variables for a time then yank them later?

@kevinsawicki

This comment has been minimized.

Contributor

kevinsawicki commented May 24, 2016

This is highly likely to break people's CI setups (including our own) - maybe support both variables for a time then yank them later?

Just because of the S3 variables? Or something else here?

@kevinsawicki

This comment has been minimized.

Contributor

kevinsawicki commented May 25, 2016

maybe support both variables for a time then yank them later?

Added warnings for ATOM_SHELL_ env var usage 👍

@paulcbetts

This comment has been minimized.

Contributor

paulcbetts commented May 25, 2016

Just because of the S3 variables? Or something else here?

I was referring to the env var names in general, anything that could be semi-public (we can rename the IPC constant names w/o deprecations though)

@kevinsawicki

This comment has been minimized.

Contributor

kevinsawicki commented May 25, 2016

we can rename the IPC constant names w/o deprecations though

These were already all renamed in #5061

@@ -187,7 +187,7 @@ def atom_gyp():
return obj['variables']
def get_atom_shell_version():
def get_electron_version():
return 'v' + atom_gyp()['version%']

This comment has been minimized.

@zcbenz

zcbenz May 25, 2016

Contributor

The atom_gyp can also be renamed.

This comment has been minimized.

@kevinsawicki

kevinsawicki May 25, 2016

Contributor

Done 👍

@zcbenz

This comment has been minimized.

Contributor

zcbenz commented May 26, 2016

👍

@zcbenz zcbenz merged commit fa1246c into master May 26, 2016

0 of 5 checks passed

continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
electron-linux-arm Build #3306818 queued
Details
electron-linux-ia32 Build #3306819 queued
Details
electron-linux-x64 Build #3306820 queued
Details

@zcbenz zcbenz deleted the no-more-atom-shell branch May 26, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment