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

Alias list_name to dataset in entities sheet #654

Merged
merged 4 commits into from
Sep 15, 2023

Conversation

lognaturel
Copy link
Contributor

@lognaturel lognaturel commented Sep 15, 2023

Closes #652

Why is this the best possible solution? Were any other approaches considered?

The intent is to match the user-facing language of "Entity List". We discussed some alternative column names but decided that list_name has the advantage of being consistent with the choices sheet. The meanings are slightly different but very much related.

I went back and forth on the text of error messages. At first I made them generic ("Invalid name") but then I thought that would be too hard to use. I think using "entity list" in the messages is friendlier. People who have existing forms with dataset are early adopters so hopefully are in a better position to figure out the issue if they run into the message.

I also added on case-sensitive checking of reserved entity property names.

What are the regression risks?

This area is well-tested and the change is small so I don't think there are any.

Does this change require updates to documentation? If so, please file an issue here and include the link below.

getodk/docs#1651

Before submitting this PR, please make sure you have:

  • included test cases for core behavior and edge cases in tests
  • run nosetests and verified all tests pass
  • run black pyxform tests to format code
  • verified that any code or assets from external sources are properly credited in comments

@lognaturel
Copy link
Contributor Author

I've targeted a v1.12.x branch off of the current v1.12.1 release so that we can release this immediately. @lindsay-stevens if you have a chance for a quick look, that'd be great since you have the context but I also recognize I'm catching you Friday afternoon! It should be straightforward for someone else to review if that's better. Thanks!

@lognaturel lognaturel removed the request for review from lindsay-stevens September 15, 2023 19:37
@lognaturel lognaturel merged commit cb7fd2d into XLSForm:v1.12.x Sep 15, 2023
10 checks passed
@lognaturel lognaturel deleted the alias_dataset branch September 15, 2023 21:04
@lognaturel lognaturel restored the alias_dataset branch September 16, 2023 03:42
@lognaturel lognaturel deleted the alias_dataset branch November 6, 2023 19:49
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