-
-
Notifications
You must be signed in to change notification settings - Fork 260
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
Feature/separate server from site #67
Feature/separate server from site #67
Conversation
Codecov Report
@@ Coverage Diff @@
## main #67 +/- ##
=======================================
Coverage 97.98% 97.98%
=======================================
Files 26 26
Lines 545 546 +1
Branches 31 31
=======================================
+ Hits 534 535 +1
Misses 11 11
Continue to review full report at Codecov.
|
"copy-fonts": "./node_modules/.bin/copyfiles -f ./node_modules/font-awesome/fonts/*.* ./node_modules/bootstrap/dist/fonts/*.* ./td/public/fonts", | ||
"bundle-css": "./node_modules/.bin/rework-npm ./td/public/content/app.css -o ./td/public/content/threatdragon.css", | ||
"minify-css": "./node_modules/.bin/uglifycss ./td/public/content/threatdragon.css > ./td/public/content/threatdragon.min.css", | ||
"build-templates": "browserify -t [browserify-ng-html2js --module templates] ./td.site/app/templates.build.js -o ./td.site/app/templates.js", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this script needed anymore? I couldn't find templates.js
or templates.build.js
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
agreed, this can be safely removed.
It is a left over from when threat dragon was a single project before it was split off into core, desktop and webapp
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is very good to see, many thanks to @lreading . Tested it just now it works as before.
This is one of the first steps in the version 2.0 roadmap, which will make it easier to provide an API to Threat Dragon. This has been requested in some verbal conversation I have had with users, which will help them use it in CI / CD pipelines and the like
There are two errors from the code scanning. I do not think these were introduced in this pull request, they seem to have always been present. @lreading did you want to confirm this? Looks good to me for merging |
It looks to be complaining about the front-end |
Agreed, we should go ahead and merge. We will be probably be revising all this code for version 2.0 so it may not be in the scan when that time comes. |
Summary
Separate the front-end code from the back-end code
Description for the changelog
Created a separate directory at the root of the project for
td.site
, and movedtd
totd.server
. Updated all local file references including scripts inpackage.json.
Other info
I think it would make sense to move the front-end to its own directory. This will allow us to migrate the back-end code to ES6 independently from any front-end code changes (eg Vue migration, e2e tests, etc). The front-end is using browserify and is slated to be migrated to Vue in the future. Having the front-end code fully separated from the back-end allows more options when moving to ES6, using transpalation tools like babel. It will also allow for more of a pragmatic approach to refactoring the back-end, where less work will need to be done in a single sweep.
I'm very open to feedback and discussion on this.