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

Move to latest *express* 4 stable #417

Merged
merged 5 commits into from Nov 18, 2014
Merged

Conversation

Martii
Copy link
Member

@Martii Martii commented Nov 10, 2014

NOTES:

  • app.router is eol'd and is no longer needed in 4.x
  • aReq.route.params moved to aReq.params
  • Don't need app_route emulation any more

@OpenUserJs/admin , @OpenUserJs/backend , @OpenUserJs/frontend , @OpenUserJs/owners :
This needs massive testing in dev please and of course @sizzlemctwizzle approval. Thanks. :)

**NOTES**:
* `app.router` is eol'd and is no longer needed.
* `aReq.route.params` moved to `aReq.params`
* Don't need `app_route` emulation any more
@Martii Martii added CODE Some other Code related issue and it should clearly describe what it is affecting in a comment. migration Use this to indicate that it may apply to an existing or announced migration. needs testing Anyone can add this but it is primarily there for the Assignee indicating that Testers are wanted. PR READY This is used to indicate that a pull request (PR) is ready for evaluation. tracking upstream Waiting, watching, wanting. labels Nov 10, 2014
Martii added 2 commits November 17, 2014 20:55
Conflicts:
	controllers/scriptStorage.js
Resolved:
	Favor HEAD
@@ -33,8 +33,8 @@ if (isPro) {
});
}

function getInstallName(aReq) {
return aReq.route.params.username + '/' + aReq.route.params.scriptname;
function getInstallName (aReq) {
Copy link
Member Author

Choose a reason for hiding this comment

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

Hmmm merge issue here... I didn't do this.

@Martii Martii removed the needs testing Anyone can add this but it is primarily there for the Assignee indicating that Testers are wanted. label Nov 18, 2014
@Martii
Copy link
Member Author

Martii commented Nov 18, 2014

Approved by sizzle in private... this will be merged sometime tomorrow after a little bit of testing on current deployment of the current head on pro.

@Martii Martii removed tracking upstream Waiting, watching, wanting. PR READY This is used to indicate that a pull request (PR) is ready for evaluation. labels Nov 18, 2014
@Martii
Copy link
Member Author

Martii commented Nov 18, 2014

sendMeta failure during retesting... investigating.

* Unfortunately I don't see an option to keep comments. :\
* Disable minification in `sendMeta` and `sendScript`
* Missing in README.md ... added and whoops

Applies to OpenUserJS#417
@Martii Martii added the PR READY This is used to indicate that a pull request (PR) is ready for evaluation. label Nov 18, 2014
@Martii
Copy link
Member Author

Martii commented Nov 18, 2014

Btw the express-minify failure would have been hidden until production without #428 ;) :)

Martii added a commit that referenced this pull request Nov 18, 2014
Move to latest *express* 4 stable

Auto-merge... fully retested on dev.
@Martii Martii merged commit 396ea27 into OpenUserJS:master Nov 18, 2014
@Martii Martii deleted the updateToExpress4 branch November 18, 2014 19:22
@Martii Martii added tracking upstream Waiting, watching, wanting. and removed PR READY This is used to indicate that a pull request (PR) is ready for evaluation. labels Nov 18, 2014
@Martii
Copy link
Member Author

Martii commented Nov 18, 2014

Just to give some assurances for those who are apprehensive about the upgrade... during the unable to deploy time of 8ish days I was constantly on dev testing a lot of things out with express 4... which is another reason why #429 superseded all current PRs including my own... have faith that things will work out for the better especially in this projects only current prolific tester please. :)

Martii pushed a commit to Martii/UserScripts that referenced this pull request Nov 18, 2014
* Add in a layer of protection in case the server errs again ... this script as a Unit Test was fundamentally inclusive in identifying a hidden issue at OpenUserJS/OpenUserJS.org#417 :)
* Version bump
Martii pushed a commit to Martii/OpenUserJS.org that referenced this pull request Nov 23, 2014
* Bump requested node to current pro node stable
* Rename route to be less cryptic and tiered based for npm functions with alteration of affected controller with handler
* Update view to reflect new urls
* Squash some deprecation warnings in here since we changed to *express* 4 at OpenUserJS#417
@Martii Martii removed the tracking upstream Waiting, watching, wanting. label Feb 7, 2015
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 20, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CODE Some other Code related issue and it should clearly describe what it is affecting in a comment. migration Use this to indicate that it may apply to an existing or announced migration.
Development

Successfully merging this pull request may close these issues.

None yet

2 participants