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

Favicon #30

Merged
merged 3 commits into from
May 9, 2020
Merged

Favicon #30

merged 3 commits into from
May 9, 2020

Conversation

sbcgua
Copy link
Collaborator

@sbcgua sbcgua commented May 1, 2020

to #29

This one appeared to be a bit messy. At the end I started looking into this option - meaning just ignore favicon requests. Maybe ... it is simpler to remove the favicon and choose that way ?

So:

  • favicon works (nodemon, npm start, docker), added a couple of deps
  • lives in public in the root
  • along with this some cleanups in config so that TS builds stuff into ./build dir, not ./build/src - this is for good anyway. But ended up messing the PR a bit, sorry for this.

@sbcgua
Copy link
Collaborator Author

sbcgua commented May 9, 2020

maybe merge ? alternatively I can simplify it to the option I mentioned above. I actually started thinking that favicon is not that necessary for such a project and does not justify added dependencies :) But, need an opinion :)

@larshp larshp merged commit 48ab503 into abaplint:master May 9, 2020
@sbcgua sbcgua deleted the favicon branch May 9, 2020 09:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants