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

Make export constants consistent with FormPack - fix #2037

Closed

Conversation

noliveleger
Copy link
Contributor

Fixed #2030.

…stead

of 'xml' to backend when choosing 'XML values and headers'
@noliveleger noliveleger requested a review from jnm October 22, 2018 19:29
used for labels. Leave unset, or use `_default` or `None`, for
labels in the default language
* `lang`: optional; It can be:
- unset (`None`) or `formpack.constants.UNTRANSLATED` for labels in the default language (default: `None`).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I try to keep lines under 80 characters for readability. It can certainly be a pain sometimes.

Is there a docstring parser you like to use that doesn't handle line breaks within these parameter descriptions well?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jnm , I kept the default of my IDE which is 120 characters instead of 80. Nowadays, screen resolutions are wider than before. 80 is really narrow, but if you want me to go with 80. I will.

// Backend also changes `_xml` to `FormPack.constant.UNSPECIFIED_TRANSLATION`
// `FormPack.constant.UNSPECIFIED_TRANSLATION` constant equals `False`.
else if (lang === '_xml' || lang === false) {
langDescription = t('XML values and headers');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Admirable, but this wraps onto a second line:
image

Also, for every language it's the values and headers, but we probably wouldn't want to say English (en) values and headers either.

Copy link
Contributor Author

@noliveleger noliveleger Oct 23, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the suggestion? Only XML in caps?
I've seen your change. Sounds good.

jnm added a commit that referenced this pull request Oct 23, 2018
Also improves handling of formpack `UNTRANSLATED` and `UNSPECIFIED_TRANSLATION`
constants; closes #2037
@jnm
Copy link
Member

jnm commented Oct 23, 2018

The build is failing because the formpack requirement wasn't updated.

Anyway, I decided I was wrong with what I was asking for in #2030. I'm closing this in favor of #2038.

@jnm jnm closed this Oct 23, 2018
@noliveleger noliveleger deleted the 2030_export_formpack_constant_consistence branch May 22, 2020 14:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants