Skip to content

Add detail to DataTable hint in JavaSnippet and update test #1298

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

Merged
merged 5 commits into from
Dec 28, 2017
Merged

Add detail to DataTable hint in JavaSnippet and update test #1298

merged 5 commits into from
Dec 28, 2017

Conversation

mlvandijk
Copy link
Member

Summary

Add detail to DataTable hint in JavaSnippet to mention that variable names of domain object ("YourType") need to match column names in the feature file.

Details

Motivation and Context

First use of DataTable didn't quite work as expected (see #1297).
This hint may be helpful to others.

How Has This Been Tested?

Unit test for JavaSnippet was updated accordingly.
'mvn clean install' built successfully.

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue).
  • New feature (non-breaking change which adds functionality).
  • Breaking change (fix or feature that would cause existing functionality to not work as expected).

Checklist:

  • I've added tests for my code.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 81.455% when pulling ebd655e on mlvandijk:datatable-hint into 4be971a on cucumber:master.

" // E,K,V must be a scalar (String, Integer, Date, enum etc)\n";
" // E,K,V must be a scalar (String, Integer, Date, enum etc)\n" +
" // Make sure the variable names for YourType match the column names\n" +
" // in your feature file.\n";
Copy link
Contributor

Choose a reason for hiding this comment

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

I think 'The field names of YourType must match the column names' is better.

Field names instead of variables. The latter exists only in a function. The former is part of a type.

And must instead of make sure because of the rfc.

Not sure about the subject and object. Swapping might be better (if only someone told me learning grammar would allow me to discuss the use of language :) ).

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated. Hope this is better.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 81.455% when pulling 4bd8401 on mlvandijk:datatable-hint into 4be971a on cucumber:master.

@@ -24,7 +24,9 @@ public String arguments(List<Class<?>> argumentTypes) {
public String tableHint() {
return " // For automatic transformation, change DataTable to one of\n" +
" // List<YourType>, List<List<E>>, List<Map<K,V>> or Map<K,V>.\n" +
" // E,K,V must be a scalar (String, Integer, Date, enum etc)\n";
" // E,K,V must be a scalar (String, Integer, Date, enum etc)\n" +
" // Field names for YourType must match the column names in \n" +
Copy link
Contributor

@mpkorstanje mpkorstanje Dec 16, 2017

Choose a reason for hiding this comment

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

I found some code that suggests the field name don't need to be an exact match.

My Field, and myField will all match myField.

Copy link
Contributor

Choose a reason for hiding this comment

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

The PascalCaseStringConverter btw.

Copy link
Contributor

Choose a reason for hiding this comment

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

And also CamelCaseStringConverter.

So my Field will probably also work.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, I'll have a look and rewrite a little to make this clear.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 8eb47a8 on mlvandijk:datatable-hint into ** on cucumber:master**.

@mlvandijk mlvandijk added the 📖 documentation Improvements or additions to documentation label Dec 28, 2017
@mlvandijk mlvandijk merged commit 0092433 into cucumber:master Dec 28, 2017
@mlvandijk mlvandijk deleted the datatable-hint branch December 28, 2017 19:31
@lock
Copy link

lock bot commented Dec 28, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Dec 28, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
📖 documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants