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

Stop using @namespace to identify scripts #130

Closed
sizzlemctwizzle opened this issue Jun 10, 2014 · 9 comments
Closed

Stop using @namespace to identify scripts #130

sizzlemctwizzle opened this issue Jun 10, 2014 · 9 comments
Labels
DB Pertains inclusively to the Database operations. enhancement Something we do have implemented already but needs improvement upon to the best of knowledge.
Milestone

Comments

@sizzlemctwizzle
Copy link
Member

It's just ugly and unnecessary. I want to wait on merging #129 before I do this.

@sizzlemctwizzle sizzlemctwizzle added this to the 0.4 milestone Jun 10, 2014
@sizzlemctwizzle sizzlemctwizzle self-assigned this Jun 10, 2014
@Martii
Copy link
Member

Martii commented Jun 10, 2014

and lengthy on urls... I've been wondering at what point we're going to hit a pathname limitation of the server drones.

@sizzlemctwizzle
Copy link
Member Author

Also, I need to remove the .toLowerCase() nonsense on usernames since it's completely unnecessary.

pathname limitation of the server drones.

I'm not sure if there is one, but shorter urls would be very nice.

@Martii
Copy link
Member

Martii commented Jun 11, 2014

remove the .toLowerCase() nonsense on usernames

I was waiting on this issue with a lower priority because .../Marti accesses me currently on production and .../marti does not.

@sizzlemctwizzle
Copy link
Member Author

Usernames are supposed to be case-insensitive (because they have to be for S3), but we use the case that you signed up when we display it on the site somewhere. You're right though, that url should work. The case shouldn't matter.

@sizzlemctwizzle
Copy link
Member Author

Something to add to the list: make script.meta.icon a string in cases where it might be an array of strings. Then add "icon" to the list of unique metadata keys in scriptStorage.js

@sizzlemctwizzle
Copy link
Member Author

Well I did it and I only broke three scripts: 1 2 3

S3 wouldn't let me copy those files to a new path for whatever reason. Perhaps they failed to upload to S3 in the first place. I added case insensitivity for user and script names, but I totally forgot about the icon. Oh well, that can be handled another time.

Edit: Almost forgot to mention. This is totally backwards compatible with links still including the namespace. That portion of the url is just ignored. You could put any old junk there and it'd still work. Example:

@jerone
Copy link
Contributor

jerone commented Jun 22, 2014

Great work!

The old namespaced urls are all over the web. Do those get a 301 status code to circumvent a penalty by Google?

@sizzlemctwizzle
Copy link
Member Author

No but it could be added.

@jerone
Copy link
Contributor

jerone commented Jun 22, 2014

No but it could be added.

Reference issue #75

@Martii Martii added the DB label Jul 3, 2014
Martii pushed a commit to Martii/OpenUserJS.org that referenced this issue Feb 21, 2016
* Remove legacy namespace in all routes ... part of OpenUserJS#262 and OpenUserJS#130... officially EOL as per sizzle and confirmed by me... no redirects.
* Remove the patches I put in for this portion
* Create function for encoding similar to GH's... see helper `encoding`
* Using encodeURIComponent from OpenUserJS#795
* Fix bug of Fork History showing "slugged" instead of native/raw
* Fix category reference bug... redirect was set to the Object itself and not one of its properties
* Fix some more missing object identifiers in modelParser.js ... not all are validated yet and some may still be missing for future proofing
* Remove and Add some more watchpoints
* Flip successful "rating updated" for Groups to debug mode and related in `./libs/modelParser.js` ... these are generating too much log traffic OpenUserJS#430

**NOTES**
* [ ] Followup with DB migration to add `fork.utf` to existing forks otherwise they may not show up next to the bullet
* Leaving GH import alone as it still works so far ... however a watchpoint added... GH web UI always `encodeURIComponent` just about everything... will need to reaffirm existing `encodeURI` later
* The term uri is inclusive to urls as previously described in OpenUserJS#819 ... however in this context url means it could be partially encoded whereas uri means it absolutely is encoded... except for the slashes... normally those are encoded but we don't want them encoded... again following GH example. A url containing `ä` may be just that however a uri has `%E4`. You will notice that I renamed some of our identifiers from `url` to `uri`... this is on purpose to indicate that is what we are expecting.
* *express* `redirect()` uses [`Location`](http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.30) and [`Content-Location`](http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.14) headers. This requires all urls to be at least URI encoded as per specs... e.g. all the browsers are adding in an `escape` for the section symbol *which may be an issue with them*. So hence why the naming of `url` vs `uri`.
* The base issue of readability of the code is still there... but at least this punches out some of the bugs mentioned.
* Compression/optimization hasn't occurred yet
* No DB migration is necessary now... this includes fixing `Re:` linkage
* The smarter encoder isn't finished yet as I may be rearranging this a bit but...

**This should knock out the BLOCKING label I put on**... at least from my perspective.

Applies to OpenUserJS#819 and OpenUserJS#262... treads on OpenUserJS#262 (comment) among many others.
Martii pushed a commit to Martii/OpenUserJS.org that referenced this issue Apr 15, 2017
* This goes back to when I first came into the project and noticed that `marti` and `Marti` were inconsistent in script access
* Added in a WARNING note for no error handling again

Applies to OpenUserJS#83 and OpenUserJS#180 / OpenUserJS#130 / OpenUserJS#130 (comment)
Martii added a commit that referenced this issue Apr 15, 2017
* This goes back to when I first came into the project and noticed that `marti` and `Marti` were inconsistent in script access
* Added in a WARNING note for no error handling again

Auto-merge

Applies to #83 and #180 / #130 / #130 (comment)
@OpenUserJS OpenUserJS locked as resolved and limited conversation to collaborators Apr 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
DB Pertains inclusively to the Database operations. enhancement Something we do have implemented already but needs improvement upon to the best of knowledge.
Development

No branches or pull requests

3 participants