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 legacy code, PROJECT_PATH and related test case code_tests #5429

Merged
merged 4 commits into from Apr 6, 2017

Conversation

Projects
None yet
2 participants
@benjaoming
Member

benjaoming commented Apr 4, 2017

Summary

Removes broken code in 'setup' management command

Targeting for 0.17.1 because there are no current plans for 0.18, and this is just a legacy cleanup.

TODO

  • Has documentation been written/updated?
  • Have you written release notes for the upcoming release?

Reviewer guidance

As noted in the release notes, PROJECT_PATH was not even configurable. It was used wrongly, too. Its default value was the KALITE_HOME environment, but some places of usage assumed it would be a "project root", which is a remnant of when KA Lite was distributed as a static directory.

Issues addressed

#4104

@benjaoming benjaoming added the has PR label Apr 4, 2017

@benjaoming benjaoming added this to the 0.17.1 milestone Apr 4, 2017

@codecov-io

This comment has been minimized.

Show comment
Hide comment
@codecov-io

codecov-io Apr 4, 2017

Codecov Report

Merging #5429 into 0.17.x will increase coverage by <.01%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           0.17.x    #5429      +/-   ##
==========================================
+ Coverage   51.76%   51.77%   +<.01%     
==========================================
  Files         143      143              
  Lines        7499     7496       -3     
==========================================
- Hits         3882     3881       -1     
+ Misses       3617     3615       -2
Impacted Files Coverage Δ
kalite/settings/base.py 88.67% <ø> (-0.11%) ⬇️
...ite/distributed/management/commands/screenshots.py 0% <0%> (ø) ⬆️
kalite/distributed/management/commands/setup.py 0% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b438691...97b4a18. Read the comment docs.

codecov-io commented Apr 4, 2017

Codecov Report

Merging #5429 into 0.17.x will increase coverage by <.01%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           0.17.x    #5429      +/-   ##
==========================================
+ Coverage   51.76%   51.77%   +<.01%     
==========================================
  Files         143      143              
  Lines        7499     7496       -3     
==========================================
- Hits         3882     3881       -1     
+ Misses       3617     3615       -2
Impacted Files Coverage Δ
kalite/settings/base.py 88.67% <ø> (-0.11%) ⬇️
...ite/distributed/management/commands/screenshots.py 0% <0%> (ø) ⬆️
kalite/distributed/management/commands/setup.py 0% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b438691...97b4a18. Read the comment docs.

@benjaoming benjaoming changed the title from Fix up some legacy code, removing PROJECT_PATH to Remove legacy code, PROJECT_PATH and related test case code_tests Apr 4, 2017

else:
start_script_path = os.path.realpath(
os.path.join(settings.PROJECT_PATH, "..", "bin", "windows", "kalite.bat"))
else:

This comment has been minimized.

@benjaoming

benjaoming Apr 5, 2017

Member

This part of the cleanup makes sense because PROJECT_PATH wasn't set to a directory containing the bin/ directory, but rather then ~/.kalite directory as we discontinued having the source checkout as user data directory.

@benjaoming

benjaoming Apr 5, 2017

Member

This part of the cleanup makes sense because PROJECT_PATH wasn't set to a directory containing the bin/ directory, but rather then ~/.kalite directory as we discontinued having the source checkout as user data directory.

This comment has been minimized.

@benjaoming

benjaoming Apr 5, 2017

Member

In other words: It's removing code that would be broken anyways.

@benjaoming

benjaoming Apr 5, 2017

Member

In other words: It's removing code that would be broken anyways.

@benjaoming benjaoming merged commit 4d3336c into learningequality:0.17.x Apr 6, 2017

2 of 3 checks passed

codecov/patch 0% of diff hit (target 51.76%)
Details
ci/circleci Your tests passed on CircleCI!
Details
codecov/project 51.77% (+<.01%) compared to b438691
Details

This was referenced Apr 6, 2017

@benjaoming benjaoming deleted the benjaoming:project_path branch Apr 6, 2017

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