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

Add option to skip release check #7445

Merged
merged 2 commits into from Aug 17, 2016

Conversation

Projects
None yet
6 participants
@c9s
Contributor

c9s commented Jul 21, 2016

Summary

In issue #7026, the release updater is not a part of the catalog update, therefore it still runs the updater even if METEOR_OFFLINE_CATALOG is specified.

The reason that we really really need this option is, meteor runs way too slow on travis-ci, it spents almost 7 minutes to just prepare the application.

Changes

A --no-release-check option is added to the run command in this PR.

@meteor-bot

This comment has been minimized.

meteor-bot commented Jul 21, 2016

@c9s: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Meteor Contributor Agreement here: https://contribute.meteor.com/

@c9s

This comment has been minimized.

Contributor

c9s commented Jul 21, 2016

I'm open to discuss the API and the option name. Please send me the feedbacks then I can update this PR.

@abernix

This comment has been minimized.

Member

abernix commented Jul 21, 2016

This provides a flag but doesn't provide any functionality for that flag. Did you forget to commit part of your fix?

@c9s

This comment has been minimized.

Contributor

c9s commented Jul 21, 2016

Just one moment, I will update the changes

Jesse Rosenberger notifications@github.com 於 2016年7月21日 星期四寫道:

This provides a flag but doesn't provide any functionality for that flag.
Did you forget to commit part of your fix?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#7445 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AADGztrbqNArfpFHSz4ms1m628NMsv38ks5qXwfggaJpZM4JRVYZ
.

@c9s

This comment has been minimized.

Contributor

c9s commented Jul 21, 2016

There was one missing commit. I've updated the changes.

@c9s c9s changed the title from Add --no-release-check option to skip release check to #7026: Add option to skip release check Jul 23, 2016

@@ -375,7 +375,7 @@ function doRunCommand(options) {
oplogUrl: process.env.MONGO_OPLOG_URL,
mobileServerUrl: utils.formatUrl(parsedMobileServerUrl),
once: options.once,
noReleaseCheck: options['no-release-check'],
noReleaseCheck: options['no-release-check'] || process.env.METEOR_NO_RELEASE_CHECK,

This comment has been minimized.

@abernix

abernix Jul 26, 2016

Member

I'm not sure this deserves an environment variable. Why wouldn't this just be a --flag?

(I realize that Meteor has a lot of other ENV variables and it's a common way to set things, however I think ENVs should be reserved for production environments. This particular flag wouldn't have any significance in production).

This comment has been minimized.

@c9s

c9s Jul 26, 2016

Contributor

I am fine to remove the environment variable. I added this because sometimes we need to run one or more meteor commands on the development side, if we have the env var, we don't have to append the option on every meteor commands.

for now I can see the number of env variable used in meteor is around 8~11

@nathantreid

This comment has been minimized.

nathantreid commented Jul 30, 2016

I would very much prefer having the environment variable, as I would have it globally enabled. I love updating to the latest version of Meteor ASAP, but production apps are typically a version or 2 behind for stability, and having to wait for Meteor to update when I need to get work done on an existing app is not good.

@abernix abernix changed the title from #7026: Add option to skip release check to Add option to skip release check Aug 3, 2016

@abernix

This comment has been minimized.

Member

abernix commented Aug 4, 2016

Ok, got around to testing this – LGTM, even if I'm not sure I agree with the ENV variable – though I suppose it's fine (btw, I suggested my eventual solution in #7559).

I tested this in what I believe to be the most reasonable way:

  • Installed Meteor 1.3.5.1 (using older version intentionally so there's something to upgrade)
  • Created a Meteor 1.3.5.1 project
  • Checked out release-1.3.5
  • Rebased this simple PR onto that
  • Changed the update runner to check for updates every 30 seconds (instead of 3 hours)
  • Added some debug just to make sure things are working right.
  • Replaced my official 1.3.5.1 installation with the checked-out version (so it would identify as an actual release, not a checkout)
  • Ran my 1.3.5.1 project I created above with the --no-release-check argument
  • Waited an intentionally-long 10 minutes! ☕
  • Confirmed that there was no network activity background downloading during that time and that the meteor-tool symlink was not updated. Debug message also confirmed that the updater was not running at all (or even scheduled).
  • Reinstalled 1.3.5.1 from install.meteor.com
  • Changed the update runner to run every 30 seconds manually
  • Confirmed that there WAS network activity after 30 seconds, and when the download was done a prompt to install Meteor 1.4.0.1, plus the symlink was changed.

Obviously if you run any of your projects without --no-release-check then the global tool will still be updated, but as per usual the app itself will only be updated if you run meteor update within that directory.

@abernix

This comment has been minimized.

Member

abernix commented Aug 4, 2016

Alternatively, it sounds like disabling auto-updates for everyone would preferred by some folks: #7026 (comment) (Though I think Internet Explorer adopted that approach at one point and it failed miserably)

@benjamn benjamn merged commit 1a8a117 into meteor:devel Aug 17, 2016

3 checks passed

CLA Author has signed the Meteor CLA.
Details
ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@benjamn benjamn added this to the Release 1.4.1 milestone Aug 17, 2016

@c9s c9s deleted the kaneoh:pr/no-release-check branch Aug 17, 2016

@c9s c9s restored the kaneoh:pr/no-release-check branch Aug 17, 2016

@c9s

This comment has been minimized.

Contributor

c9s commented Aug 17, 2016

Thanks!

benjamn added a commit that referenced this pull request Aug 17, 2016

Add option to skip release check (#7445)
* Add --no-release-check option to skip release check

* Add METEOR_NO_RELEASE_CHECK environment variable
@DominikGuzei

This comment has been minimized.

DominikGuzei commented Aug 23, 2016

Thanks to the 7 gods, new and old … this is the best "feature" lately! 👍

abernix added a commit to abernix/meteor that referenced this pull request Sep 21, 2016

Add support for `--no-release-check` to `meteor test` command
This functionality for the `test` command was overlooked when meteor#7445 was implemented.

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