search

growth-astro / growth-too-marshal

GROWTH Target of Opportunity Marshal
MIT License
13 stars 12 forks source link

add manual page danger flag #135

Open mcoughlin opened 3 years ago

mcoughlin commented 3 years ago

Does this pull request make any changes to the database? None

Code changes that affect the database require special attention for data migration. N/A

lpsinger commented 3 years ago

As I suggested before, please see if you can implement these checks as validators in the WTForm definition so that the error messages are all presented consistently.

mcoughlin commented 3 years ago

So we do this currently by actually making the schedule with the windows and references and everything... is the suggestion to run the schedule, check if empty, and then allow to continue if it passes? The validation isn't as simple as "this parameter is set correctly..."?

mcoughlin commented 3 years ago

Ah, I see, this is a bit different for the manual one. Lemme see sorry.

lpsinger commented 3 years ago

Oh, I see. It sounds like these checks don't fit naturally into the validation stage, then.

lpsinger commented 3 years ago

The unit test failures look unrelated, but we'll need to fix them in a separate PR.

mcoughlin commented 3 years ago

In a reversed form (keys and values switched). I think that set could be pulled to the top of that function.

lpsinger commented 3 years ago

Please rebase. The unit tests are now passing on main.