Skip to content
This repository has been archived by the owner on Jul 30, 2019. It is now read-only.

Fringe Page #266

Merged
merged 3 commits into from
Oct 22, 2015
Merged

Fringe Page #266

merged 3 commits into from
Oct 22, 2015

Conversation

mmmavis
Copy link
Member

@mmmavis mmmavis commented Oct 15, 2015

This will fix #178.

I will squash the commits after I get a r+

@Pomax
Not sure what the best way is to review this PR -- I could share the underlying testing Google Form and Spreadsheet with you, as well as my .env file that contains the meta of the testing Google Form. Let me know!

I'm also getting this Warning: owner-based and parent-based contexts differ React warning (from react-formation) that I'm not able to solve. Hopefully you can lend a hand 😁
screen shot 2015-10-19 at 4 04 41 pm

@mmmavis mmmavis changed the title (WIP) Fringe Page Fringe Page Oct 19, 2015
"habitat": "0.4.1",
"hatchet": "0.3.2",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removing this since we are not using hatchet in the code anymore

@ScottDowne and I hold different opinions on whether or not to leave this line in package.json... and neither of us knew what the best practice is. 😛 @Pomax thoughts?

His view:

I think it's just about expectations of what package.json is used for being consistent
If in a year time I was tasked with sending an email from mozfest site, I wouldn't have expected to see a package missing, I would expect to go to .env for that sort of config
but, it's technically not going to break anything, so it's just a matter of expectations and best practice

My preference is to keep the codebase up-to-date -- remove code and modules that aren't being used anymore. If for any reason we want to get "old" code back we use commit history and GitHub tickets to recall our lost memory. And I also think in a year time we are probably gonna switch to use another module (vs hatchet) 🐹

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I misunderstood, I didn't know the code that used hatchet was already gone.

I thought it was code we were not using at the moment, but still existed in the repo, as in a send-email endpoint that we were just not hitting anymore, but would be again next year. Taking a closer look, hatchet isn't even used in the repo. Totally remove it if that's the case.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yay problem solved! apologies if I didn't explain it clearly in the first place 😜

@mmmavis
Copy link
Member Author

mmmavis commented Oct 22, 2015

went through this with @Pomax yesterday. merging it noooow

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants