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

Feat remove duplicate title #101

Closed

Conversation

SaketThota
Copy link
Contributor

  1. Made the main title hyperlink
  2. Removed duplicate title

@@ -0,0 +1,468 @@
<!DOCTYPE html>
<html lang="en">
Copy link
Member

Choose a reason for hiding this comment

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

WHAT? why this file?

Copy link
Member

Choose a reason for hiding this comment

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

@SaketThota I don't think you meant to commit this, please amend your commits to remove it.

@@ -1,6 +1,4 @@
= OTC CatchUp Summaries
Our Tech Community
Copy link
Member

Choose a reason for hiding this comment

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

It won't matter much but why are we removing this?

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 don't think we should remove that line. That is the author of the page.

@@ -1,4 +1,4 @@
= OTC CatchUp #{catchup_display_number} Summary
= link:/summary/{catchup_display_number}[OTC CatchUp #{catchup_display_number} Summary]
Copy link
Member

Choose a reason for hiding this comment

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

Who would click on this link?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah this is not required in the individual summary.

@@ -1,11 +1,12 @@
[#{catchup_display_number}]
=== link:/summary/{catchup_display_number}[OTC CatchUp #{catchup_display_number}]
:leveloffset: -1
Copy link
Member

Choose a reason for hiding this comment

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

What this does?

Comment on lines -1 to -2
[#{catchup_display_number}]
=== link:/summary/{catchup_display_number}[OTC CatchUp #{catchup_display_number}]
Copy link
Member

Choose a reason for hiding this comment

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

As far as I remember we don't need to make any changes in this

Copy link
Member

Choose a reason for hiding this comment

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

Yes, this needs to be kept in.

@@ -71,6 +71,7 @@ done;
# Build combined summary site
asciidoctor \
-a webfonts! \
-a building_combined_summary \
Copy link
Member

Choose a reason for hiding this comment

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

Also, what is this?🤔

@SirusCodes
Copy link
Member

@SaketThota are you still working on this?

@SaketThota
Copy link
Contributor Author

@SaketThota are you still working on this?

I think I fixed the problem of having double lines before About OTC .
But now that you stated the changes, sure I can work on them and will raise a PR!

@HarshKapadia2
Copy link
Member

Okay I am really confused. Is this PR still valid @SirusCodes?

@SirusCodes
Copy link
Member

The issue is not fixed... So yeah it is valid.

@SaketThota you are still working on it?

@SaketThota
Copy link
Contributor Author

I am not currently not working on it, but would like to take it up later.

@HarshKapadia2
Copy link
Member

I think it would be better to open a new PR at this point. There are too many unnecessary changes here.

@tusharnankani
Copy link
Member

What's the status/updates of this PR?

@HarshKapadia2
Copy link
Member

@SaketThota @SirusCodes pinging y'all to look into this PR.

@SirusCodes
Copy link
Member

I will handle it in #113

@HarshKapadia2
Copy link
Member

Okay, then I'm closing this PR.

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