-
Notifications
You must be signed in to change notification settings - Fork 12
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
Configuração via YAML #16
base: master
Are you sure you want to change the base?
Conversation
attr_accessor :alt_token | ||
def config | ||
@config ||= Config.new | ||
@config |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Acho que essa linha é desnecessária, o operador ||=
já retorna o valor de @config
@danieldocki, muito bom o initializer. Você poderia fazer o Squashing dos commits: Além disso adiciona uma linha em branco no final dos arquivos como o Git pede. |
# Only +production+ for now. | ||
attr_accessor :environment | ||
def configure(&block) | ||
@config ||= Config.new |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
não seria melhor reutilizar o método config
já definido aqui ?
@cirdes @brunoluigi Obrigado! Farei as mudanças :) |
added option to use yml or initializer added javascript adapter to initializer template added environment to pagseguro yaml template added config to working with yaml normalize test to working with new configs normalize request to working with configs added adapter_javascript_url to direct access added new comments better markdown refactored config and configure methods Normalize all the line endings
@cirdes @brunoluigi Seria isso? Apanhei um pouquinho para fazer Squashing kkk |
@danieldocki, o squashing é isso mesmo. Você poderia fazer um rebase com o master? Adicionei bastante coisa no README. Acho que a gente deveria remover o "adapter_javascript_url" do initializer, é melhor incentivar que as pessoas baixam e coloquem na pasta de vendor. Mas acho importante deixar claro que é preciso incluir a lib. Você pode fazer essas alterações para gente mergiar? |
@cirdes, e se o pagseguro mudar algo no javascript ele ficará com uma versão desatualizada, mas ela realmente não faz parte da api transparente mas é necessário, acho que uma menção no README já basta. |
@danieldocki, é verdade. |
@cirdes
Adicionei generators para o Rails Initializer e YAML.
Seria isso? ou pode melhorar?