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

Partially fixes #298 - make apps pluggable #299

Open
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
3 participants
@djmgit
Member

djmgit commented Aug 11, 2017

Partially fixes #298 - made apps pluggable

Partially fixes issue - #298, moved the following apps'
dependencies into their individual folders and updated
script and css references.

```
barchart
boilerplate
bubblecharts
collegeElections
compareTwitterProfiles
CountryTweetMap
ducphanduy
emojiheatmap
emojiheatmapper
fossasia-histogram
histogram
knowthediff
LQL
MultiLinePlotter

```

I have:

  • There is a corresponding issue for this pull request.
  • Mentioned the Issue number in the pull request commit message Fixes #<number> commit message
  • There is only strictly only one commit per issue.

For the reviewers

I have:

  • Reviewed this pull request by an authorized contributor.
  • The reviewer is assigned to the pull request.
@vibhcool

vibhcool suggested changes Aug 12, 2017 edited

May be I will be wrong, but IMO images in the apps shall also be moved to images directory in the respective apps.

@kavithaenair

This comment has been minimized.

Show comment
Hide comment
@kavithaenair

kavithaenair Aug 13, 2017

Member

@djmgit I think this PR will cause redundancy and space wastage.
As you can see the PR loklak/loklak_server#802, was to remove redundancy, but this PR may again cause the same issue.

Member

kavithaenair commented Aug 13, 2017

@djmgit I think this PR will cause redundancy and space wastage.
As you can see the PR loklak/loklak_server#802, was to remove redundancy, but this PR may again cause the same issue.

@djmgit

This comment has been minimized.

Show comment
Hide comment
@djmgit

djmgit Aug 15, 2017

Member

@kavithaenair Its true that this will introduce redundancy, presently the apps have got their dependencies distributed in different folders (like js and css in root directory), so the apps are not standalone. The aim of this PR is to make the apps standalone so that any one can use an app independently (simply copy the app folder and use it somewhere else without caring about the dependencies). But that is only possible when the apps have all their dependencies with them (In the same folder) or else users will also have to manage the app dependencies as well that is also take care of the root js, css and images folder which contains dependencies of all apps.
If we use some dependency management tool like bower then too we will have to maintain bower components in each directory which will again give rise to the same issue - redundancy.

@Achint08 @hemantjadon @vibhcool @kavithaenair @singhpratyush @SKrPl what do you all suggest?

Member

djmgit commented Aug 15, 2017

@kavithaenair Its true that this will introduce redundancy, presently the apps have got their dependencies distributed in different folders (like js and css in root directory), so the apps are not standalone. The aim of this PR is to make the apps standalone so that any one can use an app independently (simply copy the app folder and use it somewhere else without caring about the dependencies). But that is only possible when the apps have all their dependencies with them (In the same folder) or else users will also have to manage the app dependencies as well that is also take care of the root js, css and images folder which contains dependencies of all apps.
If we use some dependency management tool like bower then too we will have to maintain bower components in each directory which will again give rise to the same issue - redundancy.

@Achint08 @hemantjadon @vibhcool @kavithaenair @singhpratyush @SKrPl what do you all suggest?

@vibhcool

This comment has been minimized.

Show comment
Hide comment
@vibhcool

vibhcool Aug 27, 2017

Member

IMHO Space shall not be much of the issue. If this PR keeps each app independent of each other and the website code and app code is kept separately, then this PR is fine.

Member

vibhcool commented Aug 27, 2017

IMHO Space shall not be much of the issue. If this PR keeps each app independent of each other and the website code and app code is kept separately, then this PR is fine.

Partially fixes #298 - made apps pluggable
Partially fixes issue - #298, moved the following apps'
dependencies into their individual folders and updated
script and css references.

```
barchart
boilerplate
bubblecharts
collegeElections
compareTwitterProfiles
CountryTweetMap
ducphanduy
emojiheatmap
emojiheatmapper
fossasia-histogram
histogram
knowthediff
LQL
MultiLinePlotter

```

@djmgit djmgit changed the title from [WIP] fixes #298 - make apps pluggable to Partially fixes #298 - make apps pluggable Sep 2, 2017

@djmgit

This comment has been minimized.

Show comment
Hide comment
@djmgit

djmgit Sep 2, 2017

Member

Test link: klakapps.surge.sh

@kavithaenair @vibhcool open for review.
I will be working on the remaining apps in subsequent PRs.

Member

djmgit commented Sep 2, 2017

Test link: klakapps.surge.sh

@kavithaenair @vibhcool open for review.
I will be working on the remaining apps in subsequent PRs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment