Skip to content

Fixes #3351 - Moves the analytics comment#3357

Merged
karlcow merged 1 commit intowebcompat:masterfrom
bijjybox:3351/1
Jun 24, 2020
Merged

Fixes #3351 - Moves the analytics comment#3357
karlcow merged 1 commit intowebcompat:masterfrom
bijjybox:3351/1

Conversation

@bijjybox
Copy link
Copy Markdown
Contributor

Fixes - #3351

@karlcow karlcow requested a review from ksy36 June 22, 2020 14:23
@karlcow
Copy link
Copy Markdown
Member

karlcow commented Jun 22, 2020

@apsychogirl you might want to adjust the PR, to just modify what needs to be modified. I have the feeling you got a formater which changed everything in the initial code.

@ksy36
Copy link
Copy Markdown
Contributor

ksy36 commented Jun 22, 2020

Thanks for looking into this @apsychogirl . Looks good to me, though you might want to check Karl's comment

Copy link
Copy Markdown
Member

@karlcow karlcow left a comment

Choose a reason for hiding this comment

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

so a couple of things…

  1. The title of the PR needs to be Fixes #3351 - Moves the analytics comment.
  2. The issue #3351 doesn't talk about removing the comment. It says move the comment two lines up.
  3. The commit messages need to have the proper format referencing the issue number. for example, git commit -m 'issue #3351 - moves the Google Analytics comment' webcompat/templates/layout.html

Could you fix this.

@karlcow karlcow linked an issue Jun 23, 2020 that may be closed by this pull request
@bijjybox bijjybox changed the title Removed the google analytics comment Fixes #3351 - Moves the analytics comment Jun 23, 2020
@karlcow
Copy link
Copy Markdown
Member

karlcow commented Jun 23, 2020

@apsychogirl yes! that's it. one last thing.

Could you do

git rebase -i HEAD~3
  1. replace pick by f in front of the last two lines
  2. changes the message of the first line by issue #3351 - moves the Google Analytics comment after the pick and the sha1

Then

git push -f origin 3351/1

Thanks.

@bijjybox
Copy link
Copy Markdown
Contributor Author

bijjybox commented Jun 23, 2020

image

Just a doubt, if I change the message of the first line, won't that merge the commit with issue #3354 ?
This is the output of -
git rebase -i HEAD~3

@karlcow
Copy link
Copy Markdown
Member

karlcow commented Jun 23, 2020

hmmm not the right git rebase. Yup DO NOT DO that :)

@karlcow
Copy link
Copy Markdown
Member

karlcow commented Jun 24, 2020

@apsychogirl could you tell me what

git log --oneline

returns for the first 5 lines.

@bijjybox
Copy link
Copy Markdown
Contributor Author

dfc27405 (HEAD -> 3351/1) Removed the google analytics comment
b9ba8a96 Issue #3354 - fixes syntax
3f8d764d Issue #3354 - complete the template tests
5c1d1a8a issue #3354 - convert fully to pytest
5e5b7d6b Issue #3354 - Handles FileNotFoundError for assets

@karlcow
Copy link
Copy Markdown
Member

karlcow commented Jun 24, 2020

@apsychogirl I'm confused.

dfc27405 (HEAD -> 3351/1) Removed the google analytics comment

but your latest commit says:

2dcdf7a Fixes #3351 - moves the Google Analytics comment 

oh wait… you are not committing into your branch.

@karlcow
Copy link
Copy Markdown
Member

karlcow commented Jun 24, 2020

ah no that's not it. Sorry about that.
How come the local log doesn't reflect the latest two commits?

@karlcow
Copy link
Copy Markdown
Member

karlcow commented Jun 24, 2020

@apsychogirl you should see these 3 commits
https://github.com/apsychogirl/webcompat.com/commits/3351/1

@karlcow
Copy link
Copy Markdown
Member

karlcow commented Jun 24, 2020

@apsychogirl do you want to try to fix it? or do you want me to fix your pull request?

@bijjybox
Copy link
Copy Markdown
Contributor Author

I'll try to find the error and will let you know

@bijjybox
Copy link
Copy Markdown
Contributor Author

@karlcow , does this seems fine?

@karlcow
Copy link
Copy Markdown
Member

karlcow commented Jun 24, 2020

Thanks a lot @apsychogirl
congratulations on making the PR square!

@karlcow karlcow merged commit a36807e into webcompat:master Jun 24, 2020
@bijjybox bijjybox deleted the 3351/1 branch June 28, 2020 11:54
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.

"End Google Analytics" comment in wrong place

4 participants