-
Notifications
You must be signed in to change notification settings - Fork 2
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
Remove needless "subjects" field from Compromisso content type #100
Conversation
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.
Aprovado; mas talvez seja prudente esperar um tempo para mesclar.
a0dbe58
to
88a4f84
Compare
@rodfersou vou esperar ate o final da semana; já avisamos por todos canais possíveis. @idgserpro comentários? |
Sou contra remover o campo. Eu entendi que a motivação de #88 era pra passar a usar o behavior do Plone para Pra mim esse é o caminho correto: esconde o campo, faz um upgradeStep que verifica se esse campo está sendo usado ao longo do portal naquele site em específico, e, caso esteja sendo usado, passa a usar o behavior padrão do Plone e tem os campos migrados do campo antigo para o novo: se não está sendo usado, não seta o behavior padrão do Plone. Aí, numa nova versão depois dessa que escondeu os campos, remove os campos definitivamente (pois eles já foram migrados). |
@idgserpro faz por favor um levantamento entre teus clientes que usam a agenda; não adianta ser contra algo que remove uma coisa que ninguém está usando, ou que está usando de forma errada, de acordo aos relatos na lista de emails. por outro lado, adicionar esse behavior é uma coisa que pode ser feita por qualquer pessoa competente que esteja integrando esse pacote num portal: o atributo fico em aguardo dos resultados do levantamento. |
Fiz esse teste: peguei a branch 1.x de brasil.gov.agenda. Adicionei o egg cssselect como dependência (para evitar erro no plone.protect). Criei uma agenda, uma agenda diária e um compromisso, adicionando duas tags em cada um desses tipos. Depois, dei checkout para essa branch |
@idgserpro vocês fizeram o levantamento entre seus cliente como sugerido acima? alguém está usando essa funcionalidade? |
Não consigo ter esse levantamento no tempo hábil que você colocou na lista. De qualquer forma, não concordo em remover devido ao bug apresentado em #100 (comment). O correto é remover, atribuir o behavior e migrar esses dados. Resumo: voto pra manter como está, se for pra remover os atributos nesse PR, deve ser feito o upgradeStep para migração já que só adicionando o behavior não funciona, mesmo tendo o mesmo nome. |
@idgserpro alterei o escopo do pull request para remover só o campo é importante lembrar que os compromissos não são mostrados por separado e não são também não buscáveis. no futuro, para simplificar ainda mais, esse tipo de conteúdo deveria ser completamente removido e substituído por anotações no dia da agenda do mesmo jeito que funciona o collective.liveblog. |
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.
LGTM
a4f0ac5
to
ea79e88
Compare
ea79e88
to
197e6e4
Compare
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.
OK.
See: https://listas.interlegis.gov.br/pipermail/plonegov-br/2018-June/004767.html
closes: #88