Skip to content

Conversation

hegner
Copy link
Member

@hegner hegner commented Nov 10, 2015

No description provided.

@jouvin
Copy link
Contributor

jouvin commented Nov 10, 2015

As a general remark, try to respect casing for product/service names. For example, github should be written GitHub. A global replace is needed basically: I don't add it as a comment to each instance!

get_involved.md Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

not content-> no content (not that the subject itself is useless...).

Copy link
Member Author

Choose a reason for hiding this comment

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

agreed.

@jouvin
Copy link
Contributor

jouvin commented Nov 10, 2015

The same remark about casing applies toindico. Should beIndico`.

howto.md Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

Before opening a pull request, one must push back to its fork...

Copy link
Member Author

Choose a reason for hiding this comment

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

thanks. makes more sense now.

@hegner
Copy link
Member Author

hegner commented Nov 10, 2015

I incorporated the proposals and fixed the GitHub / Indico naming.

howto.md Outdated
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: Despite this is not the recommended option, if you are not confortable with Git and you only want to do simple changes, the GitHub web interface allows you to add and edit the files in your browser. In this case, you don't need to have a local clone of your personal forkon GitHub.`

@jouvin
Copy link
Contributor

jouvin commented Nov 10, 2015

Also, in the Adding to the newsletter section, the directory mentioned is incorrect: workinggroups -> newsletter

@hegner
Copy link
Member Author

hegner commented Nov 10, 2015

Let me know once you are done w/ the review. I'll wait with my next push till then.

@jouvin
Copy link
Contributor

jouvin commented Nov 10, 2015

In the first sentence in the page, I'd replace HSF GitHub repository people by HSF GitHub contributors (making only contributors a link).

@jouvin
Copy link
Contributor

jouvin commented Nov 10, 2015

I'd also replace: We all like to work in code editors; this lets you write content in a friendly editor using the easy markdown syntax (which is used by GitHub itself). by The site content is written using the Markdown syntax.

(markdown is another word that must be capitalized!).

@jouvin
Copy link
Contributor

jouvin commented Nov 10, 2015

I think I'm done for now! Thanks again for the work done!

@jouvin
Copy link
Contributor

jouvin commented Nov 10, 2015

One more thing! front-matter is mentioned several times. It'd be good in how to add and edit to add a few word on the fact that all files starts with a few special lines delimited by --- lines than contain metadata about the page and are called front-matter.

@jouvin
Copy link
Contributor

jouvin commented Nov 10, 2015

And one last thing: the page title in the front-matter is incorrect: newsletter shoud be removed.

@hegner
Copy link
Member Author

hegner commented Nov 10, 2015

Incorporated your additional comments; should we merge?

@jouvin
Copy link
Contributor

jouvin commented Nov 10, 2015

It looks much better! I have still a few (minor remark):

  • GitHub’s Pages service: service should not be part of the anchor (in general we tend to put to many words in the anchor, IMO).
  • Pages uses Jekyll link: Pages and Jekyll should be 2 separate links
  • HSF website repository: HSF should not be part of the anchor
  • liquid in side menu customization should be a link
  • The liquid template engine used by Jekyll: only liquid should be a link
  • liquid should be Liquid
  • markdown should be Markdown (at least one occurence left at the end)
  • Markdown syntax: only Markdown should be in the anchor (2 occurences at least)

Should be perfect then!

@jouvin
Copy link
Contributor

jouvin commented Nov 10, 2015

And as usual one more thing! I'd renamed the file howto to howto-contribute in order to avoid future possible conflicts (also requires editing reference to it).

@hegner
Copy link
Member Author

hegner commented Nov 10, 2015

Can you make the last point a bug report?

@hegner hegner closed this Nov 10, 2015
@jouvin
Copy link
Contributor

jouvin commented Nov 10, 2015

Why did you close the PR?

@jouvin jouvin reopened this Nov 10, 2015
@hegner
Copy link
Member Author

hegner commented Nov 10, 2015

Did I close? By accident then.

@jouvin
Copy link
Contributor

jouvin commented Nov 10, 2015

One last detail that you missed in my previous comment: the first reference to Liquid is not a link and should be one IMO. Then I am happy to merge!

@hegner
Copy link
Member Author

hegner commented Nov 10, 2015

Done

@jouvin
Copy link
Contributor

jouvin commented Nov 10, 2015

Rename request tracked as a separate issue (#5), as requested by @hegner .

@jouvin
Copy link
Contributor

jouvin commented Nov 10, 2015

Thanks for all the refinements done! Merging...

jouvin added a commit that referenced this pull request Nov 10, 2015
Add more explanations about how to add information to the website
@jouvin jouvin merged commit eeef0e6 into HSF:master Nov 10, 2015
@hegner hegner deleted the howto_edit branch November 10, 2015 20:11
@hegner
Copy link
Member Author

hegner commented Nov 10, 2015

Thanks.

hegner pushed a commit that referenced this pull request Nov 12, 2015
Update 2015-11-08-tracking.md
valassi added a commit that referenced this pull request Mar 15, 2016
emanueleusai added a commit to emanueleusai/hsf.github.io that referenced this pull request Mar 31, 2021
jlsmith-hep added a commit to jlsmith-hep/hsf.github.io that referenced this pull request Feb 11, 2025
Co-authored-by: Valentin Volkl <valentin.volkl@cern.ch>
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.

2 participants