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

En picks/land more #226

Merged
merged 8 commits into from
Mar 20, 2014
Merged

En picks/land more #226

merged 8 commits into from
Mar 20, 2014

Conversation

unho
Copy link
Member

@unho unho commented Mar 20, 2014

No description provided.

@dwaynebailey
Copy link
Member

GTM

julen and others added 8 commits March 20, 2014 18:32
@julen @unho
Added missing submission type attribute
e451b26 
Bulk actions performed offline were missing a value for this attribute.
We're adding it for consistency and completeness. Includes data migration
code.

Pick note: When this commit was picked from Evernote fork it suffered some
changes, like replacing the migration with code in the upgrade system, and
thus increasing the build number.
@julen @unho
Added ALLOWED_HOSTS to the template development settings
cb94cb5
@iafan @unho
Make tipsy tooltips fully opaque
7cee662
@iafan @unho
sorttable.js can perfectly deal with multiple thead rows
3e603e4 
(assuming the first thead row contains the actual headers and the others contain non-sortable 'header' entries)
@iafan @unho
Use CSS selectors for even/odd row coloring in translation table
9834182 
Conflicts:
	pootle/static/css/style.css
	pootle/static/js/editor.js
@iafan @unho
Under Windows, the "nl_langinfo" method is not available
659a626
@iafan @unho
Show spinner in the middle of edit row
af98de5
@iafan @unho
Use try/except instead of checking the function existence
6110627
@unho unho merged commit 6110627 into translate:master Mar 20, 2014
@unho unho deleted the en-picks/land-more branch March 20, 2014 17:36
@julen
Copy link
Contributor

julen commented Mar 21, 2014

I wonder why would you want to replace the data migration with code in the upgrade system?

@unho
Copy link
Member Author

unho commented Mar 21, 2014

@julen See #227

@julen
Copy link
Contributor

julen commented Mar 21, 2014

Apart from being some new guidelines that doesn't explain the why I'm afraid.

@unho
Copy link
Member Author

unho commented Mar 21, 2014

@julen Sorry. Will try to explain again:

  • Avoid having that kind of code scattered all over Pootle, so it is easier to find it.
  • Despite migrations might work perfectly for you, for us might not work always (fresh installs, upgrades not from previous version are scenarios you don't face). So putting these bits that alter DB data on the upgrade ensure that we can upgrade step by step, knowing for sure nothing weird will happen. Migrations don't provide that level of certainty unless you add dependencies between them, which complicates the thing.
  • Another minor reason is that using upgrade instead of migrations decreases the amount of code, making it easier to review and understand.

You are right that the guidelines in #227 actually don't hold the reasons, but instead are the result of these reasons above, among others.

@julen
Copy link
Contributor

julen commented Mar 21, 2014

If that works for you I won't object, just notice that you have extra work converting data migrations to separate procedures.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@unho @dwaynebailey @julen @iafan