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.

Sign up for GitHub

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

Jump to bottom

Remove unused field from Project model #4092

Closed
stsewd opened this issue May 15, 2018 · 7 comments
Closed

Remove unused field from Project model #4092

stsewd opened this issue May 15, 2018 · 7 comments
Labels
Good First Issue Good for new contributors
Milestone
Cleanup

Comments

@stsewd
Copy link
Member

stsewd commented May 15, 2018

When removing the comments app from the codebase in #3802. A field was missing to be removed, we need to remove this and make a migration.

https://github.com/rtfd/readthedocs.org/blob/696a25f9b3350bb047caf1899a876aa1aa0fcbaa/readthedocs/projects/models.py#L140-L143
@stsewd stsewd added the Good First Issue Good for new contributors label May 15, 2018
@varunotelli varunotelli mentioned this issue May 15, 2018
Closed
@agjohnson agjohnson added this to the Cleanup milestone May 31, 2018
@Alig1493
Copy link
Contributor

Alig1493 commented Jun 3, 2018

I would like to take up this issue if no one else is doing it.

@stsewd
Copy link
Member Author

stsewd commented Jun 3, 2018

@Alig1493 thanks for the interest, but there is a PR in progress already #4097

@Alig1493
Copy link
Contributor

@stsewd the PR seems closed for the time being. Will it be alright for me to look into it at this moment?

@stsewd
Copy link
Member Author

stsewd commented Jul 26, 2018

@Alig1493 sure, feel free to send a PR

@Alig1493
Copy link
Contributor

Alig1493 commented Jul 26, 2018 •
edited
Loading

@stsewd I have already made some progress on this issue. Just a couple of questions regarding the test cases pertaining to these two fields:

  • should they be removed?
  • should they be changed accordingly?

If it's the latter then I might end up needing some assistance in resolving them.

Thank you in advance.

@Alig1493
Copy link
Contributor

@stsewd I've made a PR here #4431
Any guidance regarding this would be much appreciated.

@stsewd
Copy link
Member Author

stsewd commented Jul 26, 2018

@Alig1493 reviewing!

@Alig1493 Alig1493 mentioned this issue Jul 29, 2018
Merged
ericholscher added a commit that referenced this issue Oct 31, 2018
@ericholscher
Merge pull request #4431 from Alig1493/unused_comment_field
c6ee38c 
Fix for issue #4092: Remove unused field from Project model
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Good First Issue Good for new contributors
Projects
None yet
Milestone
Cleanup
Development

No branches or pull requests
3 participants
@agjohnson @stsewd @Alig1493