Skip to content
This repository has been archived by the owner on Aug 17, 2023. It is now read-only.

Test area filter in Seeding Report #223

Draft
wants to merge 15 commits into
base: main
Choose a base branch
from

Conversation

qnhn22
Copy link

@qnhn22 qnhn22 commented Apr 19, 2023

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.

@qnhn22 qnhn22 marked this pull request as ready for review April 19, 2023 18:36
@johnmaccormick johnmaccormick self-requested a review April 20, 2023 18:36
@johnmaccormick
Copy link
Collaborator

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
Copy link

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
Copy link
Collaborator

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 braughtg marked this pull request as draft April 21, 2023 15:37
@braughtg
Copy link
Member

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!

@nguyenbanhducA1K51 nguyenbanhducA1K51 deleted the seeding-report branch April 27, 2023 18:15
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants