Skip to content
This repository has been archived by the owner on May 25, 2021. It is now read-only.

ICS calendar feature #468

Closed
wants to merge 2 commits into from
Closed

Conversation

sjhuang26
Copy link
Contributor

@sjhuang26 sjhuang26 commented Jul 11, 2019

Fixes #464.
Manually copied in an external library, because of dependency issues.

@@ -38,7 +38,8 @@ services:
- "4200:4200"
volumes:
- "./web/src:/usr/src/app/src"
command: "ng build --watch"
- "./web/npm-cache:/root/.npm"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the reason for this change?

COPY ./src $APP_DIR

VOLUME /usr/src/npm-cache
RUN npm install --depth 0
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is problematic. The dependencies should be installed before the source is copied. Otherwise, the image has to be entirely rebuilt for every code change.

@Bad-Science
Copy link
Member

@sjhuang26 Thanks for the PR! A few things:

  1. What is the reason for the build configuration and image changes? I don't see how that is related to this issue.
  2. We really shouldn't be copying libraries into our codebase. Further, this code is copied without attribution or inclusion of an original license, so unless it is public domain we would definitely be violating it's license by including it this way.
  3. It would be really nice if the event repetition had a start and end time that reflected the term the section belongs to.

"npm": "^5.4.2",
"rxjs": "^6.5.2",
"rxjs-compat": "^6.5.2",
"typings": "^2.1.1",
"web-animations-js": "^2.3.1",
"yacs-api-client": "^1.0.0-beta.11",
"zone.js": "^0.8.29"
"zone.js": "^0.8.29",
"lodash": "^4.17.11",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need lodash?

@sjhuang26 sjhuang26 closed this Jul 25, 2019
@sjhuang26 sjhuang26 deleted the ics-feature branch July 25, 2019 22:06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enhance ics export
2 participants