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.

Sign up for GitHub

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

Jump to bottom

Legacy route detected for GH importing #468

Closed
Martii opened this issue Dec 2, 2014 · 9 comments
Closed

Legacy route detected for GH importing #468

Martii opened this issue Dec 2, 2014 · 9 comments
Milestone
#262

Comments

@Martii
Copy link
Member

Martii commented Dec 2, 2014

Recently just discovered at http://localhost:8080/users/marti/github ... it might be useful to list that at https://openuserjs.org/user/add/scripts or do something else with it.
@Martii Martii mentioned this issue Dec 2, 2014
Merged
@jerone
Copy link
Contributor

jerone commented Dec 2, 2014

Wondering if someone used/tested it?

@Martii
Copy link
Member Author

Martii commented Dec 2, 2014

That would be part of a PR to make sure it works before advertising it. :)

@Martii Martii added this to the #262 milestone Dec 2, 2014
@Martii
Copy link
Member Author

Martii commented Dec 2, 2014

From @Zren

The route should be removed since I rewrote it to handle pagination and the like but sizzle wanted it left there.

I can see this being a potential DDoS just feature wise and prone to user error with some complaints... so if it should be removed then it should be as per #262 ... people are allowed to change their minds down the road... rename the issue to be more specific when the assignee takes this over please. Added to the #262 milestone but it is not blocked by it.

@Zren
Copy link
Contributor

Zren commented Dec 4, 2014

require('controllers/user.js').userManageGitHubPage is the controller for the /account/github route. When this controller is removed, make sure to also remove the user.userManageGitHubPageUrl = '/account/github'; in modelParser.js. Also be sure to delete libs/repoManager.js as this controller is the only one left that references it.

@Martii
Copy link
Member Author

Martii commented Dec 4, 2014

@Zren
You can still vote and submit distinct issued PRs too. It would go a long way to reestablishing your rank by showing some more initiative. :)

@Zren Zren mentioned this issue Dec 7, 2014
Closed
@Martii Martii changed the title Orhpaned route detected for GH importing Orphaned route detected for GH importing Jan 22, 2015
@Martii
Copy link
Member Author

Martii commented Jan 22, 2015

Also be sure to delete libs/repoManager.js as this controller is the only one left that references it.

Are you sure @Zren ? See controllers/scriptStorage.js#L404 from current HEAD.

@Zren
Copy link
Contributor

Zren commented Jan 22, 2015

Thats because my refactoring of that function hasnt been merged.

@Martii
Copy link
Member Author

Martii commented Jan 22, 2015

I figured that. Please don't recommend on things that aren't merged. Everything should be off of the current HEAD not a PR.

@Martii Martii self-assigned this Jan 22, 2015
@Martii Martii changed the title Orphaned route detected for GH importing Legacy route detected for GH importing Jan 22, 2015
Martii pushed a commit to Martii/OpenUserJS.org that referenced this issue Jan 22, 2015
Remove legacy GH batch import route
96ef151 
* This route doesn't include all GH repositories so it's usefulness is very limited
* Remove associated controller
* Remove associated view
* Remove reference in `.../modelParser.js`
* Remove route
* `libs/repoManager.js` is still in use at https://github.com/OpenUserJs/OpenUserJS.org/blob/3a282c66452f895cd1f70871bbb9526885753a87/controllers/scriptStorage.js#L404 at the current HEAD for that file

**NOTES**:
* Retested existing/current GH import and is okay.
* Not tested with sync ... after retesting on pro will close the referenced issue

Affects OpenUserJS#468
@Martii Martii mentioned this issue Jan 22, 2015
Merged
@Martii
Copy link
Member Author

Martii commented Jan 22, 2015

Closing as sync on pro works. If we need a different, more up to date, landing page there open an issue for it please. Thanks for your assistance Zren.

@Martii Martii closed this as completed Jan 22, 2015
@Martii Martii removed their assignment Jan 22, 2015
@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.
Assignees
No one assigned
Labels
None yet
Milestone
#262
Development

No branches or pull requests
3 participants
@jerone @Martii @Zren