Use configuration['encoding'], because database configuration use not charset but encoding. #7572

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
3 participants
Contributor

kennyj commented Sep 8, 2012

When investigating for #7564, I found potentially bug. In database.yml, we use not charset but encoding.

That's weird, I thought it could be due to the refactoring/extraction, but 3-2 uses the same in databases.rake, which is confusing. Other databases such as postgresql also name it encoding. I think you're right, we should stick with encoding, since it's the correct option being generated by the application templates, but I'm wondering how it's being used all this time without being noticed =(.

Thanks @kennyj.

Owner

rafaelfranca commented Sep 9, 2012

Maybe because nobody use this option. Also, as far I remember I think that @kennyj added the encoding option to PostgreSQL only to master.

Contributor

kennyj commented Sep 10, 2012

@carlosantoniodasilva @rafaelfranca I added the collation option to PostgreSQL only to master (not encoding) :)

I also guess nobody use this option. Because in many case, we use utf-8 which is default.

https://github.com/rails/rails/blob/3-2-stable/activerecord/lib/active_record/railties/databases.rake#L44-48

Owner

rafaelfranca commented Sep 10, 2012

Right. Sorry for my bad memory.

I'll merge this one. Could you add a changelog entry and also create a pull request to 3-2-stable?

kennyj added a commit to kennyj/rails that referenced this pull request Sep 11, 2012

Backported #7572 to 3-2-stable. Use config['encoding'], because datab…
…ase configuration use not charset but encoding.
Contributor

kennyj commented Sep 11, 2012

@rafaelfranca I submited to 3-2-stable, and I added a changelog entry to 3-2-stable.
I think I don't need to add a changelog entry to master. Ok?

Contributor

kennyj commented Sep 11, 2012

I'll close this PR, because this diff is added to #7564.

@kennyj kennyj closed this Sep 11, 2012

rafaelfranca added a commit that referenced this pull request Sep 12, 2012

Merge pull request #7603 from kennyj/fix_charset_vs_encoding_32
Backported #7572 to 3-2-stable. Use config['encoding'], because database configuration use not charset but encoding.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment