Skip to content
This repository has been archived by the owner on Feb 22, 2020. It is now read-only.

Add links to project structure documentation #56

Closed
wants to merge 3 commits into from
Closed

Add links to project structure documentation #56

wants to merge 3 commits into from

Conversation

JasonAizkalns
Copy link

@JasonAizkalns JasonAizkalns commented Aug 16, 2017

Description

Adds links to the project structure documentation.

Reference to official issue

Addresses issue #15

Motivation and Context

Turns the references to the technologies into useful links to those technologies.

How Has This Been Tested?

Clicked on links.

CLA

  • I have signed the CLA; if other committers are in the commit history, they have signed the CLA as well

@JasonAizkalns JasonAizkalns changed the title add links to project structure documentation Add links to project structure documentation Aug 16, 2017
@reubano
Copy link
Contributor

reubano commented Aug 17, 2017

Hi @JasonAizkalns thanks for the PR! It appears some of the links you've added have stray spaces in them, e.g., [ Title](link ). Would you mind tidying those up? Also, MD formatted links apparently don't work inside tables. Not sure the best way to approach that. Maybe inlining html would work? Ideas @lamby?

@JasonAizkalns
Copy link
Author

JasonAizkalns commented Aug 17, 2017

Thank you @reubano. Let me know if cleaning the whitespace and hanging parentheses changes things for the MD formatted links. Interestingly enough, when I use atom and the markdown preview package things looks fine:

screenshot

If that still doesn't work, I'll try the inline html approach.

Aside, do you know where to find the PR template? It's mentioned in a few places, but I have not come across the appropriate formatting?

Finally, please note that I am still getting used to contributing to repos that are not my own, so I apologize for what's likely a messy commit (the .dcm files flooding my workspace?)

@lamby
Copy link
Contributor

lamby commented Aug 17, 2017

Hm, this PR/commit seems to add a bunch of git-lfs related files which appears to be a mistake (or is not otherwise documented...)

@reubano
Copy link
Contributor

reubano commented Aug 17, 2017

Interestingly enough, when I use atom and the markdown preview package things looks fine:

That is weird, maybe it's an issue with GHFMD.

screen shot 2017-08-17 at 5 24 28 pm

@reubano
Copy link
Contributor

reubano commented Aug 17, 2017

apologize for what's likely a messy commit (the .dcm files flooding my workspace?)

hmm, looks like commit #ed1693f00 is the culprit. Try a git rebase -i master to see if you can clean it up.

@lamby
Copy link
Contributor

lamby commented Aug 18, 2017

@JasonAizkalns Can you rebase this branch and remove the .dcm / git-lfs changes? :)

@JasonAizkalns
Copy link
Author

@lamby and @reubano I believe the rebase worked and I was able to drop #ed1693f00. I did not bother to reformat the links as inline html until we can confirm that the issue is specific to GHFMD.

@reubano
Copy link
Contributor

reubano commented Aug 22, 2017

did not bother to reformat the links as inline html until we can confirm that the issue is specific to GHFMD.

@lamby any chance you can verify that the links look good in the build?

@lamby
Copy link
Contributor

lamby commented Aug 24, 2017

@JasonAizkalns @reubano

They don't render here for me, alas. Dumb question, but can't we just use <a/> links?

@lamby
Copy link
Contributor

lamby commented Aug 25, 2017

@JasonAizkalns I see you pushed a change to move to <a/> in f8b2095

How come we went with the markdown ones in the first place? Just curious how you tested this? :)

@lamby
Copy link
Contributor

lamby commented Aug 28, 2017

@JasonAizkalns Friendly ping on this? :)

@JasonAizkalns
Copy link
Author

@lamby I had simply tested using the MD preview package in atom. Attempted testing with make but no success. At this point, probably worth closing this request and using @eltonlaw #64 which is more complete (spelling errors, tree corrections) and appropriately tested.

@lamby
Copy link
Contributor

lamby commented Aug 29, 2017

probably worth closing this request and using @eltonlaw #64 which is more complete (spelling errors, tree corrections) and appropriately tested.

Elton, what say you? You closed #64, but do you want to try and put something together with this one?

@eltonlaw
Copy link
Contributor

Sure, I'll reopen the old pull request. Are there any changes you want to add to the current commit?

@lamby
Copy link
Contributor

lamby commented Aug 30, 2017

@eltonlaw wrote:

Are there any changes you want to add to the current commit?

Not that I can immediately see, no :)

Thanks!

@lamby
Copy link
Contributor

lamby commented Sep 3, 2017

Hey all, what's this blocking on? Should I be looking for another commit/PR/issue? :)

Best wishes,

— Chris

@lamby
Copy link
Contributor

lamby commented Sep 5, 2017

Chris Lamb wrote:

Hey all, what's this blocking on? Should I be looking for another commit/PR/issue? :)

@eltonlaw @JasonAizkalns @reubano ^^ ;)

@reubano
Copy link
Contributor

reubano commented Sep 6, 2017

@lamby... #83 should supersede this PR. I guess we can leave this one open until the other is merged.

@lamby
Copy link
Contributor

lamby commented Sep 7, 2017

@reubano Okay, am assigning to you. :)

@lamby lamby assigned reubano and unassigned JasonAizkalns Sep 7, 2017
@reubano
Copy link
Contributor

reubano commented Sep 11, 2017

superseded by #83

@reubano reubano closed this Sep 11, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants