NYCPlanning / data-engineering-qaqc

streamlit app for data engineering
https://edm-data-engineering.nycplanningdigital.com
1 stars 0 forks source link

standardize branch options for DevDB and FacDB #266

Closed damonmcc closed 1 year ago

damonmcc commented 1 year ago

resolves #265

changes

notes

screenshots

image image
damonmcc commented 1 year ago

hey @fvankrieken, I wrote lots of notes in the PR description since I know you wanted to really think about how we should do the branch/folder list. this is an approach I thought might be nice but could definitely see how it might be overly restrictive, especially for reviewing old data

fvankrieken commented 1 year ago

Haven't gone over the code yet, but from the description it doesn't seem like it'd be restrictive - for old branches, in our workflow those changes generally should make it into main and then ideally trigger a build, we really should only need to use non-main branches for comparison when we're actively working on those branches. Main case I can see is if someone promptly deletes a branch on merge and then for some reason we still want to compare that to main, but in that case I would say maybe we just shouldn't be deleting branches until features back on main have been tested properly (i.e. build and compare to feature branch, which should be identical)

fvankrieken commented 1 year ago

Or I guess thinking about a case like EDDE this past time - numerous merges to main from numerous feature branches, which I didn't really compare between but could see the use for as sort of forensics, if something was missing see if it was introduced at intermediate point. But regardless, think that can just be handled by not deleting feature branches until data output is where we want it to be

fvankrieken commented 1 year ago

Lgtm, I personally would like to go this route and then we can iterate if we have issues with it