You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@trafficcontrol.apache.org by GitBox <gi...@apache.org> on 2020/06/12 00:09:50 UTC
[GitHub] [trafficcontrol] ocket8888 opened a new pull request #4788: Removed unused dependencies, migrated angular from bower to npm
ocket8888 opened a new pull request #4788:
URL: https://github.com/apache/trafficcontrol/pull/4788
## What does this PR (Pull Request) do?
- [x] This PR is not related to any Issue
Removes dependencies on `es5-shim` and `json3`, which are both shims for features supported by major browsers for years. Also moves `angular`, `angular-jwt`, `angular-animate`, and `angular-bootstrap-contextmenu` from bower management into NPM management. Others are probably possible to move as well, but I thought that was enough for one PR.
## Which Traffic Control components are affected by this PR?
- Traffic Portal
## What is the best way to verify this PR?
Build and run Traffic Portal. Observe that angular's still working.
## The following criteria are ALL met by this PR
- [x] No functional changes, no tests necessary
- [x] No functional changes, no documentation necessary
- [x] An update to CHANGELOG.md is not necessary
- [x] This PR includes any and all required license headers
- [x] This PR does not include a database migration
- [x] This PR **DOES NOT FIX A SERIOUS SECURITY VULNERABILITY**
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] ocket8888 commented on pull request #4788: Removed unused dependencies, migrated angular from bower to npm
Posted by GitBox <gi...@apache.org>.
ocket8888 commented on pull request #4788:
URL: https://github.com/apache/trafficcontrol/pull/4788#issuecomment-900358849
I think the differences between this branch and master have drifted to the point where if I really wanted to do this - and at this point I'd rather give up in favor of replacing TP altogether - I'd want to start from scratch.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #4788: Removed unused dependencies, migrated angular from bower to npm
Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #4788:
URL: https://github.com/apache/trafficcontrol/pull/4788#discussion_r441044060
##########
File path: traffic_portal/bower.json
##########
@@ -1,23 +1,17 @@
{
"name": "traffic_portal",
"dependencies": {
- "angular": "1.7.8",
- "angular-animate": "1.7.8",
"angular-resource": "1.7.8",
Review comment:
Not all of "the angular* deps" can be moved over so trivially. Particularly, the bootstrap stuff doesn't load properly when included with browserify from npm. I don't know why, but it might be related to the fact that the version we use from bower is unavailable in npm. Also note, though, that neither available versions are supported, neither package manager offers supported versions, and the version we use is two major versions behind what's current.
With all that in mind I was hoping to just remove angular-bootstrap* entirely, especially if it's only used for the table context menus, since the new ag-grid tables won't use angular-bootstrap-contextmenu.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] mitchell852 commented on a change in pull request #4788: Removed unused dependencies, migrated angular from bower to npm
Posted by GitBox <gi...@apache.org>.
mitchell852 commented on a change in pull request #4788:
URL: https://github.com/apache/trafficcontrol/pull/4788#discussion_r439480001
##########
File path: traffic_portal/bower.json
##########
@@ -1,23 +1,17 @@
{
"name": "traffic_portal",
"dependencies": {
- "angular": "1.7.8",
- "angular-animate": "1.7.8",
"angular-resource": "1.7.8",
Review comment:
by leaving these other angular ones in bower.json, its still pulling angular in via `bower install`
```
angular-bootstrap#0.14.3 app/bower_components/angular-bootstrap
└── angular#1.7.8
```
so i would suggest either a) limiting this PR to just removing unused deps (es5-shim and json3) or b) moving ALL of the angular* deps to npm.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] mitchell852 commented on a change in pull request #4788: Removed unused dependencies, migrated angular from bower to npm
Posted by GitBox <gi...@apache.org>.
mitchell852 commented on a change in pull request #4788:
URL: https://github.com/apache/trafficcontrol/pull/4788#discussion_r439480001
##########
File path: traffic_portal/bower.json
##########
@@ -1,23 +1,17 @@
{
"name": "traffic_portal",
"dependencies": {
- "angular": "1.7.8",
- "angular-animate": "1.7.8",
"angular-resource": "1.7.8",
Review comment:
by leaving these other angular ones in bower.json, its still pulling angular in via `bower install`
```
angular-bootstrap#0.14.3 app/bower_components/angular-bootstrap
└── angular#1.7.8
```
so i would suggest either a) limiting this PR to just removing unused deps (es5-shim and json3) and moving the migration of all angular* to npm to another PR or b) moving ALL of the angular* deps to npm in this PR.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] ocket8888 closed pull request #4788: Removed unused dependencies, migrated angular from bower to npm
Posted by GitBox <gi...@apache.org>.
ocket8888 closed pull request #4788:
URL: https://github.com/apache/trafficcontrol/pull/4788
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org