Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create 2019-08-13-Introducing....md #72

Open
wants to merge 18 commits into
base: master
Choose a base branch
from
Open

Conversation

GeorgiaHCA
Copy link
Contributor

Adding this again so people can review it!

Adding this again so people can review it!
@GeorgiaHCA GeorgiaHCA changed the title Create 2018-08-13-Introducing....md Create 2019-08-13-Introducing....md Aug 13, 2019
Copy link
Contributor

@sgibson91 sgibson91 left a comment

Choose a reason for hiding this comment

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

Hi @GeorgiaHCA, thank you for the lovely blog! I've left a few minor comments but will approve 💖

Btw, a Markdown/GitHub hack: I prefer to start each new sentence on a new line when writing Markdown. This is easier to review on GitHub as the whole paragraph doesn't get captured in the suggested changes (as it has in my suggestions below). And Markdown knows to still render all the individual sentences as a single paragraph - it creates a new paragraph when there's a blank new line - so it will still look the same when published.

What you've done is by no means wrong, but you might find this tip useful in the future 😊

_posts/2018-08-13-Introducing....md Outdated Show resolved Hide resolved
_posts/2018-08-13-Introducing....md Outdated Show resolved Hide resolved
_posts/2018-08-13-Introducing....md Outdated Show resolved Hide resolved
_posts/2018-08-13-Introducing....md Outdated Show resolved Hide resolved
_posts/2018-08-13-Introducing....md Outdated Show resolved Hide resolved
Copy link
Contributor

@minnieho1115 minnieho1115 left a comment

Choose a reason for hiding this comment

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

Thank you Georgia!It is a cool interview!!!From your interview, I have a deep understanding of Sara and ang's work.Thank you. You did a great job.My only suggestion is the same as Sara's.Each sentence can be written as a new line:)

GeorgiaHCA and others added 6 commits August 15, 2019 12:04
Made reviewers suggestions and gave each sentence its own line. Thanks for helping everyone :-)
Copy link
Member

@KirstieJane KirstieJane left a comment

Choose a reason for hiding this comment

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

Hi @GeorgiaHCA!! Thank you for writing this up! Its really interesting - I'm glad you all got to have these conversations!

A couple of formatting things that need to change:

  • The file is in the wrong directory - it should be saved in _posts/blog/2018-08-13-Introducing....md rather than _posts/2018-08-13-Introducing....md
  • Having ..... in the file name is too confusing for jekyll so the site doesn't build by default. I also think that the name needs to be a little clearer for folks reading the blog post list. So maybe "Georgia meets three people at the Turing?" or something like that? I would also love it if you, @minnieho1115 and @sparkler0323 had similar titles so that it was clear that you've all done the same blog post exercise. I'll let you three think of something good.
  • Emojis using the :sparkles: syntax don't render unfortunately - they need to be copied and pasted from (eg) emojipedia.

image

Copy link
Member

@KirstieJane KirstieJane left a comment

Choose a reason for hiding this comment

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

Sorry for leaving the review in two parts - I clicked enter too early by accident!

Great blog - well done!

_posts/2018-08-13-Introducing....md Outdated Show resolved Hide resolved
_posts/2018-08-13-Introducing....md Outdated Show resolved Hide resolved
_posts/2018-08-13-Introducing....md Outdated Show resolved Hide resolved
_posts/2018-08-13-Introducing....md Outdated Show resolved Hide resolved
_posts/2018-08-13-Introducing....md Outdated Show resolved Hide resolved
_posts/2018-08-13-Introducing....md Outdated Show resolved Hide resolved
_posts/2018-08-13-Introducing....md Outdated Show resolved Hide resolved
_posts/2018-08-13-Introducing....md Outdated Show resolved Hide resolved
@KirstieJane
Copy link
Member

Hi @GeorgiaHCA - I know you aren't well but I'm away next week so this is just a ping to make sure you don't forget about incorporating the reviews here.

@KirstieJane
Copy link
Member

Hi @GeorgiaHCA - I've updated the master branch to fix some broken links - please can you update your branch so that both CIs pass?

@KirstieJane
Copy link
Member

Pinging again - I'd like to get this blog post merged - its been open for 2 months.

Let me know if you have any questions about updating the branch @GeorgiaHCA

@GeorgiaHCA
Copy link
Contributor Author

Should be ready to merge - thanks for all your help Kirstie!

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.

5 participants