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

Remove jQuery and jQuery UI sources from repository #13706

Merged
merged 2 commits into from Nov 28, 2017

Conversation

2 participants
@mauriciofauth
Member

mauriciofauth commented Sep 28, 2017

The jquery and jquery-ui packages includes their source codes, which makes it unnecessary to include the source codes in the repository.

@mauriciofauth

This comment has been minimized.

Show comment
Hide comment
@mauriciofauth

mauriciofauth Sep 28, 2017

Member

@nijel We can do this, right?

Member

mauriciofauth commented Sep 28, 2017

@nijel We can do this, right?

@codecov

This comment has been minimized.

Show comment
Hide comment
@codecov

codecov bot Sep 28, 2017

Codecov Report

Merging #13706 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master   #13706   +/-   ##
=======================================
  Coverage   53.78%   53.78%           
=======================================
  Files         485      485           
  Lines       82305    82305           
=======================================
  Hits        44268    44268           
  Misses      38037    38037

codecov bot commented Sep 28, 2017

Codecov Report

Merging #13706 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master   #13706   +/-   ##
=======================================
  Coverage   53.78%   53.78%           
=======================================
  Files         485      485           
  Lines       82305    82305           
=======================================
  Hits        44268    44268           
  Misses      38037    38037
@nijel

This comment has been minimized.

Show comment
Hide comment
@nijel

nijel Sep 28, 2017

Member

The source code is there for licensing reasons (in the end we release under GPLv2, so we have to distribute source code). I'm not really sure how using package manager fits into this...

Member

nijel commented Sep 28, 2017

The source code is there for licensing reasons (in the end we release under GPLv2, so we have to distribute source code). I'm not really sure how using package manager fits into this...

@mauriciofauth

This comment has been minimized.

Show comment
Hide comment
@mauriciofauth

mauriciofauth Sep 28, 2017

Member

Currently only the jquery.min.js file is being included in the release archive. Source files are not included. We could add the node_modules directory in the phpmyadmin-sources archive to anyone who wants all the files.

Member

mauriciofauth commented Sep 28, 2017

Currently only the jquery.min.js file is being included in the release archive. Source files are not included. We could add the node_modules directory in the phpmyadmin-sources archive to anyone who wants all the files.

@mauriciofauth mauriciofauth changed the title from Add jQuery to packages.json and remove jQuery sources to Add jQuery to package.json and remove jQuery sources Sep 28, 2017

mauriciofauth added some commits Sep 28, 2017

Remove jQuery source code
The jquery package includes source code, which makes it unnecessary to include
the source code in the repository.

Signed-off-by: Maurício Meneghini Fauth <mauriciofauth@gmail.com>
Remove jQuery UI source code
The jquery-ui package includes source code, which makes it unnecessary to
include the source code in the repository.

Signed-off-by: Maurício Meneghini Fauth <mauriciofauth@gmail.com>

@mauriciofauth mauriciofauth changed the title from Add jQuery to package.json and remove jQuery sources to Remove jQuery and jQuery UI sources from repository Oct 20, 2017

@mauriciofauth

This comment has been minimized.

Show comment
Hide comment
@mauriciofauth

mauriciofauth Oct 20, 2017

Member

The changes I made now were just to make this PR more specific.

Member

mauriciofauth commented Oct 20, 2017

The changes I made now were just to make this PR more specific.

@nijel nijel merged commit 80e90bf into phpmyadmin:master Nov 28, 2017

5 of 6 checks passed

continuous-integration/travis-ci/pr The Travis CI build could not complete due to an error
Details
DCO All commits have a DCO sign-off from the author
Scrutinizer Analysis: No new issues – Tests: passed
Details
codacy/pr Good work! A positive pull request.
Details
codecov/patch Coverage not affected when comparing a8cd41b...e0900ec
Details
codecov/project 53.78% remains the same compared to a8cd41b
Details
@nijel

This comment has been minimized.

Show comment
Hide comment
@nijel

nijel Nov 28, 2017

Member

In the end I've decide to merge this even though I'm not 100% sure this is okay from legal view. On the other side I have not heard opposite as well...

Member

nijel commented Nov 28, 2017

In the end I've decide to merge this even though I'm not 100% sure this is okay from legal view. On the other side I have not heard opposite as well...

@mauriciofauth mauriciofauth deleted the mauriciofauth:jquery-src branch Nov 28, 2017

@nijel nijel self-assigned this Nov 29, 2017

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