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

home.html: Wrap --pre in a code tag #216

Merged
merged 1 commit into from Nov 14, 2017
Merged

home.html: Wrap --pre in a code tag #216

merged 1 commit into from Nov 14, 2017

Conversation

gabru-md
Copy link
Contributor

For short term contributors: we understand that getting your commits well
defined like we require is a hard task and takes some learning. If you
look to help without wanting to contribute long term there's no need
for you to learn this. Just drop us a message and we'll take care of brushing
up your stuff for merge!

Checklist

  • I read the commit guidelines and I've followed
    them.
  • I ran coala over my code locally. (All commits have to pass
    individually.
    It is not sufficient to have "fixup commits" on your PR,
    our bot will still report the issues for the previous commit.) You will
    likely receive a lot of bot comments and build failures if coala does not
    pass on every single commit!

After you submit your pull request, DO NOT click the 'Update Branch' button.
When asked for a rebase, consult coala.io/rebase
instead.

Please consider helping us by reviewing other peoples pull requests as well:

The more you review, the more your score will grow at coala.io and we will
review your PRs faster!

index.html Outdated
@@ -36,6 +36,16 @@

<link rel="shortcut icon" href="favicon.ico" type="image/x-icon">
<script src="components/ngstorage/ngStorage.min.js"></script>
<style type="text/css">

Choose a reason for hiding this comment

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

type Attributes: The default type for style tags is "text/css" so it can be safely omitted: Remove the type attribute from the style tag.

Origin: HTMLLintBear, Section: html.

@gabru-md gabru-md changed the title Closes coala/frontend-landing#163 Changed pre tags to code tags Closes coala/landing-frontend#163 Changed pre tags to code tags Aug 28, 2017
Copy link
Member

@IpshitaC IpshitaC left a comment

Choose a reason for hiding this comment

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

Hi @gabru-md
Could you please modify your commit message, according to the guide? Also, since a commit reflects a single atomic change, you should squash all these commits into a single one.

index.html Outdated
@@ -36,6 +36,7 @@

<link rel="shortcut icon" href="favicon.ico" type="image/x-icon">
<script src="components/ngstorage/ngStorage.min.js"></script>
<link rel="stylesheet" type="text/css" href="resources/css/code.css">

Choose a reason for hiding this comment

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

type Attributes: The default type for link tags is "text/css" so it can be safely omitted: Remove the type attribute from the link tag.

Origin: HTMLLintBear, Section: html.

@gabru-md
Copy link
Contributor Author

gabru-md commented Sep 1, 2017

@IpshitaC I have squashed my commits into single one.
with the commit message : "Fixes #163 Changed tags"
Please Review . Thanks . :D

Copy link
Member

@IpshitaC IpshitaC left a comment

Choose a reason for hiding this comment

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

Hi @gabru-md
According to the guide to writing good commit messages, each commit message should have three distinct parts, a shortlog, the commit body and the issue reference.
Here's how you should write the shortlog. It is a short description of your change. Right now, your shortlog contains the issue reference. You could change it to something like just
Change pre to code tags in install section.
Here's a guide to writing a good commit message body. A slightly detailed description of the change you're making. Maybe something like
Encloses the -pre part with code tags.
Finally, your commit should contain the issue reference. Right now, it's in your PR title and in the shortlog. It should be shifted to this section. Something like
Closes https://github.com/coala/landing-frontend/issues/163

Here's how you can amend your commit messages.

@gabru-md gabru-md changed the title Closes coala/landing-frontend#163 Changed pre tags to code tags Changed pre tags to code tags Sep 2, 2017
@IpshitaC
Copy link
Member

IpshitaC commented Sep 2, 2017

ack 0042f71

Copy link
Member

@hemangsk hemangsk left a comment

Choose a reason for hiding this comment

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

Could you please post screenshots of the final output 👍

@gabru-md
Copy link
Contributor Author

coala1
coala2

Tags were changed on index page and about page.
So i've posted 2 screenshots.

@gabru-md
Copy link
Contributor Author

@hemangsk Please Review

@hemangsk
Copy link
Member

Hey @gabru-md , really sorry for delay in the review I was badly stuck in some university work. Actually looking at the screenshot, using code tag is definitely changing the overall feel, imo pre was better, but I think maybe Adhityaa had something else in mind? ping @adtac, could you please take a look on this when you get some time?

@adtac
Copy link
Member

adtac commented Oct 10, 2017

Oh no, I think you've misunderstood my issue. You see the "You can append --pre to that command to get the latest prerelease straight from master!" line in the homepage? The "--pre" part of the line should be within a code tag. That's it. I didn't mean to ask to change all the code elements - just change the "--pre" part to:

<code>--pre</code>

Apologies if it was unclear.

@hemangsk @gabru-md

@gabru-md
Copy link
Contributor Author

@adtac okay!
No problem I'll make the necessary changes.

@gabru-md gabru-md changed the title Changed pre tags to code tags home.html: Wrapped --pre into code tags Nov 14, 2017
@gabru-md
Copy link
Contributor Author

@IpshitaC @adtac Please see. I have made the changes.

@jayvdb
Copy link
Member

jayvdb commented Nov 14, 2017

The tone of your commit message needs to be fixed.

Use shortlog

home.html: Wrap --pre in a code tag

No need for an extra repetitive sentence in the commit body

Just a blank line and the Closes... line which you already have done.

@gabru-md gabru-md force-pushed the master branch 2 times, most recently from 3985e29 to 84d5c86 Compare November 14, 2017 05:30
@gabru-md gabru-md changed the title home.html: Wrapped --pre into code tags home.html: Wrap --pre in a code tag Nov 14, 2017
@jayvdb
Copy link
Member

jayvdb commented Nov 14, 2017

ack 450b2a4

@jayvdb
Copy link
Member

jayvdb commented Nov 14, 2017

@rultor merge

@rultor
Copy link

rultor commented Nov 14, 2017

@rultor merge

@jayvdb OK, I'll try to merge now. You can check the progress of the merge here

@rultor
Copy link

rultor commented Nov 14, 2017

@rultor merge

@jayvdb Done! FYI, the full log is here (took me 2min)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

8 participants