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

Bug 1171575 - Remove unused cancel_all function from controller.js #771

Merged
merged 1 commit into from Jul 17, 2015

Conversation

vaibhavmagarwal
Copy link
Contributor

Review on Reviewable

@tojon
Copy link
Contributor

tojon commented Jul 16, 2015

Drive by.. just to ensure we've encompassed everything for removal? :)
https://github.com/mozilla/treeherder/search?utf8=%E2%9C%93&q=cancel

@vaibhavmagarwal
Copy link
Contributor Author

I am not sure about other areas, since I have not read through all of TH code :)
But I have confidence, that this function is not used.

@edmorley
Copy link
Contributor

Whilst this function is unused, I wonder if in fact this is the better location for the similar functions that are used? ie: in the future should we support "cancel all" for non-buildbot? If so, the functionality needs to be more generic than just in buildapi.js

I'll defer to @camd on this, since iirc he wrote the cancel job code :-)

@vaibhavmagarwal
Copy link
Contributor Author

The cancelAllJobs() function that is currently being used is in jobs.js:

$scope.cancelAllJobs = function(revision) {

@camd
Copy link
Contributor

camd commented Jul 17, 2015

After taking a look (gosh, we have a lot of cancelAll type of funcitons) it looks like the one @vaibhavmagarwal mentions in jobs.js is the main one. It, in turn calls other model/service functions to do its work.

But this one in question was added long ago by me and, I think, never used. Having it in the plugins/controller.js would be the wrong place anyway.

Good find, @vaibhavmagarwal ! :)

camd pushed a commit that referenced this pull request Jul 17, 2015
Bug 1171575 - Remove unused cancel_all function from controller.js
@camd camd merged commit 972e889 into mozilla:master Jul 17, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants