Skip to content

Conversation

@MeliaBurnley
Copy link
Contributor

@MeliaBurnley MeliaBurnley commented Aug 3, 2018

This is a preliminary pass at creating the Hedgehog level of the Web Application Development core skill.

We're gathering thoughts/opinions on the criteria for this level, as well as any additional reading material we could include to help engineers learn what's required for this level

@MeliaBurnley MeliaBurnley changed the title WIP - Web application development Web application development Aug 3, 2018
Learning the fundamentals - How not to hurt yourself with React

- [ ] Can demonstrate an understanding of the benefits of using React
- [ ] Creates an application with the following:
Copy link
Member

Choose a reason for hiding this comment

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

Do we have to use create-react-component or would say using Next.JS be acceptable?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Either is fine - we didn't want to tie anyone to using create-react-app when they may be more familiar with Next.JS

In theory, I'd like to see us create The Made Tech Way with some ideas about what we'd recommend as standard, but the core skills should be independent of that

- The candidate has setup the project with the following tools:
- [ ] A sandbox environment to quickly prototype components within (e.g. Storybook)
- [ ] Hot-reloading
- [ ] Tests (The ability to write them, writing one is not a requirement at this stage)
Copy link
Member

Choose a reason for hiding this comment

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

How do you assess that the tool has been set up correctly without writing a failing test 😛 ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Depends what route you go, but I'd say the app running on localhost is enough to show your tooling is set up, and a single test would show your test framework is correct. If you go the create-react-app route, it'll have a basic test included. Wouldn't that suffice without any further input from the candidate?

Copy link
Member

Choose a reason for hiding this comment

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

You can test that your test framework is running by writing a dummy failing test

e.g. expect(1).to eq(2)

Copy link
Contributor

Choose a reason for hiding this comment

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

Probably worth having that as a requirement, but would the autogenerated one in create-react-app suffice?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would say yes - the requirement mainly is "Do you actually know how to start a React project with tests" - if your approach to that was "Use create-react-app" then that's fine

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Possibly this requirement should be:

'Tests - Which can be run using make test (evidenced by a failing test)'

Copy link
Contributor

Choose a reason for hiding this comment

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

I like this approach @danielburnley


- [ ] Can demonstrate an understanding of the benefits of using React
- [ ] Creates an application with the following:
- [ ] The application can be run on any machine (e.g. Using Docker)
Copy link
Member

Choose a reason for hiding this comment

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

An odd one in my opinion, are we explicitly asking for Docker support? What was the intent behind this one?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

From memory the intent was that we wanted to make sure people knew how to set up a React project that could be worked on by multiple people. Potentially you could know how to create one but it was tied to some globally installed package on your machine so no one else could pick it up.

It does seem a bit strange to have it in here though on second reading 🤔 It depends on whether or not we feel the ability to set up a project to run anywhere is a general requirement of all our work and shouldn't be tested here? Hmm

Copy link
Member

Choose a reason for hiding this comment

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

How else are you going to get make serve to just work on any machine that has docker installed

Copy link
Member

Choose a reason for hiding this comment

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

In that case I’d reword it as:

The application has make recipies for serve and test which only depend on the host machine having Docker installed.

- [ ] The use of props within components
- [ ] The use of state within components
- [ ] The use of children within components
- [ ] Responding to user events
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, not sure how I'd explain this one.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd discuss things like registering handlers on the onSubmit/onClick methods of components

@adrianclay
Copy link
Member

Loving the direction this is taking. Feels like the Hedgehog section is more theoretical, and fox practical. Wondering why you took that direction.

- [ ] How to test component rendering
- [ ] How to test business logic

**Coming Soon - Fox is intended to test ability to write simple components and understanding
Copy link
Member

Choose a reason for hiding this comment

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

I'd also question the lack of cohesion between understanding architecture and the ability to demonstrate it. In my eyes, it'd feel more natural to explain why alongside implementing,but I'd like to hear your reasoning.

Copy link
Contributor

Choose a reason for hiding this comment

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

For me, quite a lot of the how is still different from project to project - we're still evolving our process as we go. Knowing we want to split business logic from rendering and why, and how we test this, comes before having actually done it potentially.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I think Knowing we want to split business logic from rendering is a general knowledge thing. It's driven from knowing this is a pattern that applies to whatever tooling you use.

Pulling it off in react is a specific skill that requires putting your knowledge into practice.

Copy link
Contributor

Choose a reason for hiding this comment

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

Are we confident we have a definitive answer on how we do that currently?

Copy link
Member

Choose a reason for hiding this comment

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

I wouldn't say we're looking for a definitive answer, but I think we have a few approaches that can work

Copy link
Contributor

Choose a reason for hiding this comment

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

If the aim is to prove you're a seasoned react dev, yes. If the aim is to get people to the level where they can contribute to React projects?

Copy link
Contributor

Choose a reason for hiding this comment

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

That said, I'm coming around to the idea of Fox being fairly heavily loaded with practical examples, I agree that examples of components, state management and styling with appropriate tests would make sense, then Owl could have room for more interesting things like business logic, APIs etc

Copy link
Member

Choose a reason for hiding this comment

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

I think by the time you've earned the top badge, then you should be setup to become a seasoned react dev in a relatively short period of time e.g. 6-12 months of practice

Copy link
Contributor

Choose a reason for hiding this comment

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

We're on the same page with this I think, it's just harder to tell with Fox & Owl being incomplete.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I think we're all on the same page, should clear up once we have fox/owl more solid. Hopefully within the next few weeks


The first two sections of React's documentation, [JSX](https://reactjs.org/docs/introducing-jsx.html) and [Rendering](https://reactjs.org/docs/rendering-elements.html) are important reading.

- [Components and Props](https://reactjs.org/docs/components-and-props.html)
Copy link
Member

@craigjbass craigjbass Aug 6, 2018

Choose a reason for hiding this comment

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

Do we want to include context in the fundamentals?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe? I was debating it - but i'm not sure whether or not it is a fundamental or a more advanced concept. You could compare it to using component composition potentially? But i'm not sure what level that would be

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Having looked into it I'm erring on the side of no? I it should be mentioned in Fox potentially, but you can get by without it so i wouldn't class it as a 'Fundamental'

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd argue the context APIs are important for any reasonably involved React app, would personally like to see them included. Given there's a good chance engineers will eventually have to use it on client projects (or should use it without knowing it) and there's a 100% chance they'll use it in libraries (e.g. redux, react router, theme providers).

Think they're often seen as dark magic, so hopefully a React course like this would be able to properly explain it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍 I will defer to you on this one as I've got less experience with them which has made me think they might be less important

Copy link
Contributor

Choose a reason for hiding this comment

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

Wouldn't that be more appropriate for Fox or Owl?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So imo for contexts:

  • It's used in a lot of tools, so it's pretty fundamental knowledge to get how they work/how you'd test them in future

But

  • It's not required for building a very basic react app

So i'm not sure whether or not I'd consider it fundamental or not

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd consider it Fox and above in that case

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

"In the typical React dataflow, props are the only way that parent components interact with their children... However, there are a few cases where you need to imperatively modify a child outside of the typical dataflow "

This doesn't sound like a fundamental to me. By the end of this core skill (at least, the first three levels if more got added) - I would expect the person to be empowered enough to be able to look up things like references and understand what they do because they have a good understanding of the React fundamentals.

We could very easily fall into the trap of believing everything is a fundamental - but we should make sure what we mark down actually are fundamental to Rect knowledge. Arguably Context isn't due to you needing to have an understanding of what props are to begin with, but a lot of tools use context which makes me think it might be worthwhile

- [ ] How to test business logic

**Coming Soon - Fox is intended to test ability to write simple components and understanding
of architecture & testing strategies for complex applications**
Copy link
Member

Choose a reason for hiding this comment

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

Test code / Production code contravariance? (this kinda thing)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

🤷‍♂️ definitely worth looking into I think when we start on the second level

- [ ] Creates an application with the following:
- The application can be run on any machine:
- [ ] The application has `make` recipes for `serve` and `test` which
only depend on the host machine having docker installed
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure about this criteria. It's good practice but seems out of context with React. Feel like being able to run a React app is good enough?

e.g. I might be upset if I spent a good amount of time learning React, but had to learn Docker as well to pass the test.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah - I'd like to have some way of being like "I can run it on my machine" but maybe just having it use package-json.lock/yarn.lock would be good? Mainly I don't want it to depend on global installed packages that aren't locked?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I do think the docker requirement is.. iffy

Copy link
Contributor

Choose a reason for hiding this comment

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

It's in line with our development standards though - agree on iffyness

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But should Core Skills reflect our development tools?

Copy link
Contributor

Choose a reason for hiding this comment

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

Probably? Setting up a react app won't help if it's not usable in production I suppose

Copy link
Contributor Author

Choose a reason for hiding this comment

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

🤔 hmm

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd say a package lock would be an acceptable solution, as would docker?

- [ ] Responding to user events
- Demonstrating understanding of React fundamentals
- The candidate can explain the following:
- [ ] How to style components
Copy link
Contributor

Choose a reason for hiding this comment

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

Style in any possible way? What's the criteria here?

Is style={{ color: 'blue' }} as good as hooking up emotion, react css modules, etc...?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe we should be specific about it not being in-line CSS? But in terms of how, I don't think it's important.

As I said above a hypothetical Made Tech Way might be more specific, but as this core skill is independent of that I don't think it matters too much

Copy link
Contributor Author

Choose a reason for hiding this comment

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

(at least I don't think inline CSS is the typical approach in React, maybe i'm wrong though)

Copy link
Member

Choose a reason for hiding this comment

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

Ha yeah I read that and immediately thought of the style prop. If you aren't wanting me to use the style prop you should call that out.

Copy link
Contributor

Choose a reason for hiding this comment

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

I had assumed style prop would be sufficient for Hedgehog, and using 'proper' methods comes under architecture in later levels


Learning the fundamentals - How not to hurt yourself with React

- [ ] Can demonstrate an understanding of the benefits of using React
Copy link
Contributor

Choose a reason for hiding this comment

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

Would like to see more specific required criteria on these, as we have on the TDD mark scheme/secret reviewers guide.

Copy link
Member

Choose a reason for hiding this comment

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

I thought that, but the why-and-when.md file does expain things.

@MeliaBurnley
Copy link
Contributor Author

Having spoken to Steve it feels like there might be a mix of expectations as to what level the engineer having achieved this core skill should be at - I wouldn't expect a person who has achieved this core skill to be a seasoned React dev, but more have the knowledge and skills to begin writing maintainable apps, with the knowledge to be able to begin looking at the more advanced features and make well-reasoned decisions on how/when to use them.

There's some risk here that this core skill could become bloated with lots of stuff if we're not careful, and not only will that potentially put people off it'll make it much harder/longer to assess. For now, I'd like to keep this PR focused on Hedgehog level, and avoid getting too sidetracked about higher up levels for the moment.

@StevenLeighton21
Copy link
Contributor

Have seen some good discussion re: theory vs practical. We've clarified the aims, and the goal of making Hedgehog quite primary and quick, and 'back loading' a lot of the actual code demonstration requirements into Fox. Would like to get this merged and focus on defining Fox so that we can see that it will suffice in terms of proving ability, with the caveat that if its too back loaded we can move some of the code out to Hedgehog. Thoughts?

Copy link
Member

@craigjbass craigjbass left a comment

Choose a reason for hiding this comment

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

Merge if issues are created for the big topics

@craigjbass
Copy link
Member

@StevenLeighton21 @danielburnley should merge since you've run Hedgehog!

@StevenLeighton21
Copy link
Contributor

Assuming you're happy with the issues raised and plan of action @craigjbass :)

@craigjbass
Copy link
Member

I'm happy to see it roll forward, retrospect + iterate

@StevenLeighton21 StevenLeighton21 merged commit fbbc99c into master Aug 15, 2018
@StevenLeighton21 StevenLeighton21 deleted the web-application-development branch August 15, 2018 15:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants