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.

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

New languages #3759

Merged
merged 14 commits into from
Nov 1, 2018
Merged

New languages #3759

merged 14 commits into from
Nov 1, 2018

Conversation

julienmalard
Copy link
Contributor

@julienmalard julienmalard commented Mar 9, 2018

I added a few Pakistani and all officially recognised Guatemalan languages. I am submitting a new pull request as I can't seem to be able to rebase my original request (ref #3369) and have a clean working tree.
I hope that this pull request will be acceptable!

@agjohnson
Copy link
Contributor

Thanks for reworking your branch! This may generate a migration, we should add that here as well if so. Does makemigrations give a project migration?

@agjohnson agjohnson added the PR: work in progress Pull request is not ready for full review label Mar 10, 2018
@julienmalard
Copy link
Contributor Author

julienmalard commented Mar 10, 2018 via email

@julienmalard
Copy link
Contributor Author

Hello,

I can't seem to be able to get a successful migration run on my computer (it says that I need to install packages which I have in fact installed)...any chance you could help me out with the migration?

Many thanks,
Julien Malard

@RichardLitt
Copy link
Member

@agjohnson Is there a document on how to do this, somewhere?

@humitos
Copy link
Member

humitos commented Mar 14, 2018

@RichardLitt @julienmalard I think the best place to read about this is the Django official documentation at https://docs.djangoproject.com/en/2.0/topics/migrations/

@julienmalard in case you still have problems, please paste the output of the command that it's failing so we can see what it's happening in your side.

@julienmalard
Copy link
Contributor Author

Hello all,

Many thanks for your support! I figured out the mistake (seems that some build dependencies were not explicitly listed in the requirements file).
Here is the output from the migration run. I think it means that it worked? I will push the repository now.

(venv) C:\Users\jmalar1\PycharmProjects\readthedocs.org>python manage.py makemigrations
System check identified some issues:

WARNINGS:
?: (1_8.W001) The standalone TEMPLATE_* settings were deprecated in Django 1.8 and the TEMPLATES dictionary takes precedence. You must put the values of the following settings into your default TEMPLATES dict: TEMPLATE_DEBUG.
?: (guardian.W001) Guardian authentication backend is not hooked. You can add this in settings as eg: AUTHENTICATION_BACKENDS = ('django.contrib.auth.backends.ModelBackend', 'guardian.backends.ObjectPermissionBackend').
Migrations for 'builds':
0005_auto_20180315_1604.py:
- Alter field error on build
- Alter field output on build
- Alter field state on build
- Alter field type on build
- Alter field privacy_level on version
- Alter field slug on version
- Alter field type on version
- Alter field from_slug on versionalias
- Alter field to_slug on versionalias
Migrations for 'comments':
0002_auto_20180315_1604.py:
- Alter field decision on moderationaction
- Alter field date on nodesnapshot
Migrations for 'gold':
0002_auto_20180315_1604.py:
- Alter field level on golduser
Migrations for 'integrations':
0003_auto_20180315_1604.py:
- Create proxy model BitbucketWebhook
- Create proxy model GenericAPIWebhook
- Create proxy model GitHubWebhook
- Create proxy model GitLabWebhook
- Alter field integration_type on integration
Migrations for 'oauth':
0009_auto_20180315_1604.py:
- Alter field clone_url on remoterepository
- Alter field ssh_url on remoterepository
- Alter field vcs on remoterepository
Migrations for 'projects':
0024_auto_20180315_1604.py:
- Alter field canonical on domain
- Alter field count on domain
- Alter field allow_promos on project
- Alter field comment_moderation on project
- Alter field conf_py_file on project
- Alter field default_version on project
- Alter field documentation_type on project
- Alter field has_valid_webhook on project
- Alter field language on project
- Alter field privacy_level on project
- Alter field programming_language on project
- Alter field python_interpreter on project
- Alter field repo_type on project
- Alter field suffix on project
- Alter field theme on project
- Alter field version_privacy_level on project
Migrations for 'redirects':
0002_auto_20180315_1604.py:
- Alter field redirect_type on redirect
- Alter field to_url on redirect

@agjohnson
Copy link
Contributor

@julienmalard oops, it looks like you got all of the field changes in a migration set! Make sure you run the migrate step of installation before generating and new migrations:
http://docs.readthedocs.io/en/latest/install.html

@julienmalard
Copy link
Contributor Author

julienmalard commented Mar 16, 2018 via email

@agjohnson
Copy link
Contributor

Hrm, that doesn't seem to have helped. There are still a large number of spurious migrations included.

@julienmalard
Copy link
Contributor Author

julienmalard commented Mar 21, 2018 via email

@agjohnson
Copy link
Contributor

If you can, just create a migration for the projects application and remove any unwanted migrations from that file. There are likely skipped migrations currently around help_text updates.

@agjohnson agjohnson added this to the Admin UX milestone Jun 8, 2018
@julienmalard
Copy link
Contributor Author

Better? :)

@julienmalard
Copy link
Contributor Author

Hello,
I just wanted to ask whether what I have done was correct/sufficient or if I would need to do something else for this pull request to be acceptable.
Thanks!
Julien

@stsewd
Copy link
Member

stsewd commented Jul 11, 2018

Looks like there is a problem with the migration. Can you rebase your branch to the current master and re-generate the migration? Also can you use a named migration? python manage.py makemigrations --name changed_my_model your_app_label

@julienmalard
Copy link
Contributor Author

Sure, will do that now. (By the way, does the migration have to be done on Python 2, or should it not make a difference?)
Thanks for the help!

@stsewd
Copy link
Member

stsewd commented Jul 11, 2018

It shouldn't make any difference

@humitos humitos self-assigned this Aug 16, 2018
@humitos
Copy link
Member

humitos commented Aug 16, 2018

I'm assign this to myself to merge after the migration that is taking place this weekend.

NOTE: tests are not passing, we need to rebase with master and probably update the migration.

@agjohnson
Copy link
Contributor

Unassigning for now, cleaning this PR up is going to take a bit of work and not something core team can focus on ATM.

@ericholscher
Copy link
Member

Fixed up this migration, and it should be ready to merge. 👍

@codecov
Copy link

codecov bot commented Nov 1, 2018

Codecov Report

Merging #3759 into master will increase coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #3759      +/-   ##
==========================================
+ Coverage   76.39%   76.41%   +0.01%     
==========================================
  Files         158      158              
  Lines        9980     9988       +8     
  Branches     1261     1262       +1     
==========================================
+ Hits         7624     7632       +8     
  Misses       2016     2016              
  Partials      340      340
Impacted Files Coverage Δ
readthedocs/projects/constants.py 100% <ø> (ø) ⬆️
readthedocs/projects/tasks.py 69.31% <0%> (ø) ⬆️
readthedocs/projects/admin.py 91% <0%> (+0.09%) ⬆️
readthedocs/gold/models.py 96.15% <0%> (+0.15%) ⬆️
readthedocs/gold/forms.py 95% <0%> (+0.45%) ⬆️
readthedocs/payments/forms.py 60.49% <0%> (+0.49%) ⬆️

@ericholscher ericholscher merged commit 0f40231 into readthedocs:master Nov 1, 2018
@julienmalard
Copy link
Contributor Author

Many thanks!

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.

6 participants