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
reformated catalog json slightly #169
Conversation
Pull Request Test Coverage Report for Build 107
💛 - Coveralls |
) | ||
case class StartDateRules( | ||
daysOfWeek: Option[List[DayOfWeek]] = None, | ||
selectableWindowRules: Option[SelectableWindowRules] = None, |
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.
waitch out for training commas, as it prevents the scala formatter from working
...which may be a good thing depending on your opinion 😆
| "cutOffDayInclusive": "Tuesday", | ||
| "minDaysAfterCutOff" : 20, | ||
| "windowSizeDays" : 28, | ||
| "selectableWindowRules" : { |
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.
is the word 'Rules' useful in this name?
| "windowSizeDays" : 28, | ||
| "selectableWindowRules" : { | ||
| "cutOffDayInclusive": "Tuesday", | ||
| "minDaysAfterCutOff" : 20, |
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.
now that we're in the "selectable window" context, this is actually the start of the window. So maybe startDaysAfterCutoff? or daysBetwenCutoffAndStart
great just a couple of comments now that the context has changed for the names but other than that definitely better |
👍 🚀 |
added all the window params into its own json object