Skip to content

Remove unused fiels and methods from core/models #5327

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
Closed
stsewd opened this issue Feb 20, 2019 · 6 comments
Closed

Remove unused fiels and methods from core/models #5327

stsewd opened this issue Feb 20, 2019 · 6 comments
Labels
Good First Issue Good for new contributors Improvement Minor improvement to code
Milestone
Cleanup

Comments

@stsewd
Copy link
Member

stsewd commented Feb 20, 2019

We are not using these:

https://github.com/rtfd/readthedocs.org/blob/3218c808e87206983c4eb3ce4faceddbcc0f7aa3/readthedocs/core/models.py#L35-L39

https://github.com/rtfd/readthedocs.org/blob/3218c808e87206983c4eb3ce4faceddbcc0f7aa3/readthedocs/core/models.py#L53-L67

And I don't think we are going to use them in the future, if we agreed in the removal, we can mark this as a good first issue.
@stsewd stsewd added Improvement Minor improvement to code Needed: design decision A core team decision is required labels Feb 20, 2019
@stsewd stsewd added this to the Cleanup milestone Feb 20, 2019
@stsewd stsewd added Good First Issue Good for new contributors and removed Needed: design decision A core team decision is required labels Mar 1, 2019
@ReshmaPD ReshmaPD mentioned this issue Mar 1, 2019
Merged
@ReshmaPD
Copy link

ReshmaPD commented Mar 1, 2019

Hello @stsewd,
do we just delete these specific lines and send a PR?

@stsewd
Copy link
Member Author

stsewd commented Mar 1, 2019

Yes, and any other code related to these lines. Also, you can check that all tests are passing after that. And I think you'll need to generate a migration file.

rshrc added a commit to rshrc/readthedocs.org that referenced this issue Mar 1, 2019
@rshrc
Addressed Issue readthedocs#5327
3f2f62d 
Deleted allow_email and also the function get_contribution_details() function in core/models.py
@rshrc
Copy link
Contributor

rshrc commented Mar 5, 2019

Since this issue is addressed now, should we close it?

@rshrc rshrc mentioned this issue Mar 5, 2019
Merged
@stsewd
Copy link
Member Author

stsewd commented Mar 5, 2019

#5383 isn't merged yet.

@rshrc
Copy link
Contributor

rshrc commented Mar 7, 2019

#5383 isn't merged yet.

So when will this be ready for merge?

@dojutsu-user
Copy link
Member

Hi @rshrc
Team will merge it when the time permits.
Thanks for the contribution.

stsewd added a commit that referenced this issue Mar 8, 2019
@stsewd
Merge pull request #5383 from rshrc/issue-5327
bb4c639 
Addressed Issue #5327
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 Improvement Minor improvement to code
Projects
None yet
Milestone
Cleanup
Development

No branches or pull requests
4 participants
@stsewd @rshrc @dojutsu-user @ReshmaPD