Skip to content

Refactored tasks files from project & core #3804

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

Closed
wants to merge 4 commits into from

Conversation

shubham76
Copy link
Contributor

This is regarding #3775.
Moved the mentioned tasks from projects/tasks.py to core/tasks.py.
Also made some changes in docs, tests & a few other files where the call was being made to projects/tasks.py.
Please have a look and let me know if something is wrong.

@agjohnson
Copy link
Contributor

Thanks for taking a swing at this! I think we may have missed an opportunity to talk about this first, so we likely sent you down the wrong path. Hold up on changes here for now, I've raised issues in the underlying ticket for this PR.

A few other things:

  • Moving code and altering code makes this near impossible to review, so it might be best to split up the work to refactor that location with a separate PR with the code changes you applied. Alternatively, you can move code in one commit and change code in another commit, and mention that on the PR.
  • Tests appear to be broken, but I'd wait on more guidance on the underlying ticket before continuing to resolve the errors here, as reverting this will require more code changes

@agjohnson agjohnson added PR: work in progress Pull request is not ready for full review and removed PR: ready for review labels Mar 15, 2018
@agjohnson agjohnson added this to the Cleanup milestone Mar 15, 2018
@shubham76 shubham76 mentioned this pull request Mar 16, 2018
8 tasks
@shubham76 shubham76 closed this Apr 13, 2018
@shubham76 shubham76 deleted the refactor-tasks-files branch April 13, 2018 10:10
@shubham76
Copy link
Contributor Author

Deleted the branch as there was a lot of mess in the files. Created another branch to make changes as suggested.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: work in progress Pull request is not ready for full review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants