search

DickinsonCollege / FD2School-FarmData2-S23

A fork of FarmData2 that is used for the FarmData2 School Activities.
Other
1 stars 36 forks source link

Test area filter in Seeding Report #223

Open qnhn22 opened 1 year ago

qnhn22 commented 1 year ago

Pull Request Description

Write tests to check the report table when filtering by area. Addresses #188

Licensing Certification

FarmData2 is a Free Cultural Work and all accepted contributions are licensed as described in the LICENSE.md file. This requires that the contributor holds the rights to do so. By submitting this pull request I certify that I satisfy the terms of the Developer Certificate of Origin for its contents.

johnmaccormick commented 1 year ago

Hi team (@qnhn22 , @phong260702 , @nguyenbanhducA1K51) -- I'm confused about something. Your pull request contains changes to four files, including nontrivial changes to two different .spec.js files. Is this really what you intended? I haven't spent a lot of time studying this carefully, but I think there should be exactly one relevant .spec.js file for this issue, so it's important to craft your pull request in such a way that only that one relevant change file shows up as part of the PR. Most likely, you need to base your branch off the main branch, not off one of your other feature branches. But it's also possible I have misunderstood the setup here, so feel free to reply with your own interpretation of the situation. Thanks!

nguyenbanhducA1K51 commented 1 year ago

Hi professor Maccormick I think you are right that we did not branch off the main branch. We will fix this issue by create new branch and make other pull request. By the way, could we move the file epr1.md to somewhere else not in the main branch since it does not consistent with the upstream branch and may cause unnecessary merging commit ?

johnmaccormick commented 1 year ago

By the way, could we move the file epr1.md to somewhere else not in the main branch since it does not consistent with the upstream branch and may cause unnecessary merging commit ?

@nguyenbanhducA1K51 -- Duc -- sure, good idea, I didn't think of that. Yes, you can feel free to move your epr1.md into a branch called "reports".

braughtg commented 1 year ago

We will fix this issue by create new branch and make other pull request.

Please close this PR when you have made the new one. Thanks!