GuildCrafts / floworky

1 stars 18 forks source link

bug with createRootItem #95

Closed arayi closed 7 years ago

arayi commented 7 years ago

have traced through every single part of the registration chain. sometimes it inexplicably starts throwing errors like "name" parameter must be a string when I can't find anything in the registration chain that even USES a "name" parameter.

seems that createRootItem should be a similar function to send in src/mail/registration_email.js - any idea why it is not saving?

jrob8577 commented 7 years ago

@arayi I'm confused as to why we're creating an entirely new branch and PR - the goal was to complete the feedback on the existing PR. I'm still wondering how much we're seeing as broken by starting anew, and really don't want to invest the time in re-creating an existing branch and PR... The development and testing have already been done, so I feel like we're re-inventing the wheel with this approach?

arayi commented 7 years ago

I'm not starting from scratch, just pulled out this part of the code to indicate which specific part I wanted feedback on. All I'm doing is fixing the merge conflict (i.e. trying to stay in ZPD after I'd planned out this week based on VERY different initial conditions) prior to making the PR, since that commit was made on a version of GC/master that had changes to several of the same functions in its latest commit.

Turns out I just needed a second pair of eyes, @deadlyicon helped me find a minor syntax error. The rest of the code had only minor conflicts so I'll have no trouble presenting a merge-ready PR first thing in the AM :zap:

Do you want me to do the final PR from Rachel's repo then?

jrob8577 commented 7 years ago

I would like to minimize changes, and it seems to me that final PR from original branch serves that goal.

arayi commented 7 years ago

Okay, I'll have to get access to Rachel's repo, then, but since she is at lunch right now, here are the merge-ready changes for review :)

arayi commented 7 years ago

requested changes applied