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

added mailing list #107

Merged

Conversation

akashc777
Copy link
Contributor

Signed-off-by: Akash Hadagali [email protected]

Description

This PR fixes #101

Notes for Reviewers

Signed commits

  • Yes, I signed my commits.

@welcome
Copy link

welcome bot commented May 13, 2021

Yay, your first pull request! 👍 A contributor will be by to give feedback soon. In the meantime, please review the Layer5 Community Welcome Guide and sure to join the community Slack.
Be sure to double-check that you have signed your commits. Here are instructions for making signing an implicit activity while peforming a commit.

@leecalcote leecalcote requested review from austinogiza, vinayaksh42, mertgor and DelusionalOptimist and removed request for mertgor May 14, 2021 00:07
Copy link
Contributor

@DelusionalOptimist DelusionalOptimist left a comment

Choose a reason for hiding this comment

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

Looks great @akashc777! 🚀
Just some very minor changes :D

docs/_includes/mailing-list.html Outdated Show resolved Hide resolved
@DelusionalOptimist
Copy link
Contributor

The responsive one looks kinda odd 🤔
2021-05-15_00-28

I don't know much about how UI works so feel free to ignore this if it is not in your control 🙂

@DelusionalOptimist
Copy link
Contributor

Also, you might need to do a rebase. This might be useful for reference.

@akashc777
Copy link
Contributor Author

@DelusionalOptimist I didn't receive designs for mobile view so didn't make it responsive yet 🙄

@DelusionalOptimist
Copy link
Contributor

@DelusionalOptimist I didn't receive designs for mobile view so didn't make it responsive yet roll_eyes

Oh! My bad. I should have seen it in the issue. 😬

@akashc777 akashc777 force-pushed the enhancement/mailing-list_section branch from 7ec0978 to f271dd0 Compare May 14, 2021 19:38
@akashc777
Copy link
Contributor Author

akashc777 commented May 14, 2021

I need some help with sign-off (sry I'm new to this)
not sure it worked since DCO is still red

@DelusionalOptimist
Copy link
Contributor

Hmm 🤔. Seems like applying suggestions from GitHub itself added Co-authored-by: Rudraksh Pareek <[email protected]> with the commit message instead of your sign off in 189d749 (#107).
Maybe that's why DCO is failing.

@adithyaakrishna
Copy link
Member

@akashc777 Could you run these commands, in the same order? I'm not sure they would fix it, but we can try this 👀

  • git config --global user.name "Your Name"
  • git config --global user.email "Your Email"
  • git commit --amend --no-edit --signoff
  • git push --force-with-lease origin enhancement/mailing-list_section

@akashc777
Copy link
Contributor Author

@adithyaakrishna yup... 1st two are I think one-time thing... I did run the last two in the same order

@akashc777
Copy link
Contributor Author

Hmm 🤔. Seems like applying suggestions from GitHub itself added Co-authored-by: Rudraksh Pareek <[email protected]> with the commit message instead of your sign off in 189d749 (#107).
Maybe that's why DCO is failing.

i think that's the issue

Copy link
Member

@vinayaksh42 vinayaksh42 left a comment

Choose a reason for hiding this comment

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

The mailing list component is merging with the footer. Both the footer and Mailing list components have the same background color. I am not sure if this is part of the design, but it looks odd (suggestions on this @austinogiza).
Also, the space occupied by the component is a bit larger than what we usually have for a mailing list ( for example, on this page https://layer5.io/projects/getnighthawk, both subscribe and cncf 'join us' components are taking a bit less space.)

@adithyaakrishna
Copy link
Member

adithyaakrishna commented May 14, 2021

@adithyaakrishna yup... 1st two are I think one-time thing... I did run the last two in the same order

Yeah, it's a one time thing, I thought there was an issue with the config regarding the signoffs
Saw it just now, your first commit is perfectly signedoff 😶

@akashc777
Copy link
Contributor Author

akashc777 commented May 14, 2021

The mailing list component is merging with the footer. Both the footer and Mailing list components have the same background color. I am not sure if this is part of the design, but it looks odd (suggestions on this @austinogiza).
Also, the space occupied by the component is a bit larger than what we usually have for a mailing list ( for example, on this page https://layer5.io/projects/getnighthawk, both subscribe and cncf 'join us' components are taking a bit less space.)

I followed these designs https://www.figma.com/file/5ZwEkSJwUPitURD59YHMEN/Layer5-Designs?node-id=5884%3A12909
the mailing list buttons are bigger than the join us button I think that's right?

</div>


<div class="mailinglist-buttons">
Copy link
Member

Choose a reason for hiding this comment

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

Do a simple center alignment of all three buttons for the mobile view.

Copy link
Contributor Author

@akashc777 akashc777 May 14, 2021

Choose a reason for hiding this comment

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

you mean to remove flex ?

docs/_sass/mailing-list.scss Show resolved Hide resolved
@akashc777 akashc777 force-pushed the enhancement/mailing-list_section branch from f271dd0 to cfbabfc Compare May 14, 2021 20:36
@DelusionalOptimist
Copy link
Contributor

So I'm listing some steps. Follow these to fix the DCO thingy:

  • On your branch, do git rebase -i d4c6596
  • In the editor that opens, change
pick 189d749 Update docs/_includes/mailing-list.html spelling mistakes

to

reword 189d749 Update docs/_includes/mailing-list.html spelling mistakes

and save

  • Another editor will open where you should remove the line that says
    Co-authored-by: Rudraksh Pareek <[email protected]> and save.
  • Then you need to do
    git commit --amend -s and save and quit. This should sign off this commit
  • Check if your changes are alright and the history looks aligned.
  • Force push to update changes here

Free tip, if you see vim :w to save :q to quit.

@vinayaksh42
Copy link
Member

@akashc777 akashc777 force-pushed the enhancement/mailing-list_section branch from cfbabfc to b307f5d Compare May 14, 2021 20:43
@akashc777
Copy link
Contributor Author

So I'm listing some steps. Follow these to fix the DCO thingy:

  • On your branch, do git rebase -i d4c6596
  • In the editor that opens, change
pick 189d749 Update docs/_includes/mailing-list.html spelling mistakes

to

reword 189d749 Update docs/_includes/mailing-list.html spelling mistakes

and save

  • Another editor will open where you should remove the line that says
    Co-authored-by: Rudraksh Pareek <[email protected]> and save.
  • Then you need to do
    git commit --amend -s and save and quit. This should sign off this commit
  • Check if your changes are alright and the history looks aligned.
  • Force push to update changes here

Free tip, if you see vim :w to save :q to quit.

Thanks a lot !
Now I'm not gonna use GitHub for future commits

Signed-off-by: Akash Hadagali <[email protected]>
@akashc777
Copy link
Contributor Author

@DelusionalOptimist @vinayaksh42 made it responsive for mobile please have a look

Copy link
Contributor

@DelusionalOptimist DelusionalOptimist left a comment

Choose a reason for hiding this comment

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

This looks all good to me.
Nice work @akashc777 🎉

Copy link
Member

@vinayaksh42 vinayaksh42 left a comment

Choose a reason for hiding this comment

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

Great work @akashc777

@vinayaksh42 vinayaksh42 merged commit dce653f into layer5io:master May 17, 2021
@welcome
Copy link

welcome bot commented May 17, 2021

Thanks for your contribution to the Layer5 community! 🎉

Congrats!

@leecalcote
Copy link
Member

Teamwork!

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

Successfully merging this pull request may close these issues.

[Site] Add links to GetNighthawk mailing lists
5 participants