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

Cleanup admin controller file #810

Merged
merged 2 commits into from
Nov 7, 2015

Conversation

Martii
Copy link
Member

@Martii Martii commented Nov 6, 2015

  • Don't use options for JSON and plain text outputs instead use module helper ... faster/cleaner... this is also a Unit Test in this area as well as Establish a build/exec/deploy system #249 checking
  • Added watchpoint for Fix UTF-8 Support #200 with encodeURI
  • Only time I've encounter nil helper function so far... added notation on inconsistency and since it's the only helper function used at this time in this file condensed the require
  • Added watchpoint for an encountered special render op... first I've seen of these so far... I know what the output looks like but unable to find a matching view for it other than it's visual reference in Admin Tools

NOTES

  • Questions to follow
  • One TODO to finish depending on answers

Originally applies to #262 ... not part of #262 (comment)

* Don't use `options` for JSON and plain text outputs instead use module helper ... faster/cleaner... this is also a Unit Test in this area as well as OpenUserJS#249 checking
* Added watchpoint for OpenUserJS#200 with `encodeURI`
* Only time I've encounter `nil` helper function **so far**... added notation on inconsistency and since it's the only helper function used at this time in this file condensed the `require`
* Added watchpoint for an encountered special render op... first I've seen of these so far... I know what the output looks like but unable to find a matching view for it other than it's visual reference in Admin Tools

**NOTES**
* Questions to follow
* One TODO to finish depending on answers

Originally applies to OpenUserJS#262 ... not part of OpenUserJS#262 (comment)
@Martii Martii added migration Use this to indicate that it may apply to an existing or announced migration. CODE Some other Code related issue and it should clearly describe what it is affecting in a comment. labels Nov 6, 2015
@@ -86,7 +102,7 @@ exports.userAdmin = function (aReq, aRes, aNext) {
});
});

aRes.render('userAdmin', options);
aRes.render('userAdmin', options); // NOTE: Watchpoint
Copy link
Member Author

Choose a reason for hiding this comment

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

@sizzlemctwizzle Cc: @Zren

Do either one of you know where this physical view is? The docs for express res.render (and related app.render) don't seem to cover this.

Refs:

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

Ahhh so dead code from a different code migration... Thank you!

@Martii Martii added question A question has been encountered by anyone and has remained unanswered until cleared. and removed question A question has been encountered by anyone and has remained unanswered until cleared. labels Nov 7, 2015
* Remove NOTEs about inconsistent usage... actual function module exports these out... so comment isn't quite accurate on helper function... keeping as is
* Remove dead code/comment from prior code migrations/implementations

Originally applies to OpenUserJS#262 ... not part of OpenUserJS#262 (comment)
Post fix for OpenUserJS#810
Martii added a commit that referenced this pull request Nov 7, 2015
Cleanup admin controller file

Auto-merge
@Martii Martii merged commit a25cae8 into OpenUserJS:master Nov 7, 2015
@Martii Martii deleted the cleanupAdminControllerFile branch November 7, 2015 01:16
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 16, 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.

2 participants