[MIG] website_anchor_smooth_scroll: Migration to 11.0 by njeudy · Pull Request #407 · OCA/website

njeudy · 2018-01-11T16:48:42Z

No description provided.

oca-clabot · 2018-01-11T16:48:44Z

Hey @njeudy, thank you for your Pull Request.

It looks like some users haven't signed our Contributor License Agreement, yet.
You can read and sign our full Contributor License Agreement here: http://odoo-community.org/page/website.cla
Here is a list of the users:

Alexandre Díaz (no github login found)

Appreciation of efforts,
OCA CLAbot

pedrobaeza · 2018-01-11T16:50:35Z

Can you please squash together "OCA Transbot..." + "[FIX] Remove en.po..." commits. Ref: https://github.com/OCA/maintainer-tools/wiki/Merge-commits-in-pull-requests

njeudy · 2018-01-11T16:55:53Z

@pedrobaeza yes it is done .. I also simplified history

njeudy · 2018-01-11T16:56:29Z

+               ' smooth scroll',
+    'version': '11.0.1.0.0',
+    'category': 'Website',
+    'website': 'http://www.antiun.com',


@pedrobaeza need I put oca website ?

New rule is to put github repository url (https://github.com/OCA/website)

njeudy · 2018-01-11T16:57:17Z

@@ -0,0 +1,28 @@
+# -*- coding: utf-8 -*-
+# © 2016 Antiun Ingeniería S.L. - Jairo Llopis


@pedrobaeza is this line needed I don't now if it could be delete or not, or if we need to add all contributors.

The idea is to collect here all the contributors, and when touched, on the respective files.

pedrobaeza · 2018-01-11T17:04:40Z

But I see that you have removed totally Transbot commits. You shouldn't do that, but let only one.

njeudy · 2018-01-11T17:10:13Z

@pedrobaeza I squash transbot file with 8.0 commit , 10.0 commit .. I did not drop them

I just keep all authors, and squash commit with transbot that are in relation.

pedrobaeza · 2018-01-11T17:12:15Z

I'm seeing now only 3 commits, but no trace of any "OCA Transbot..." commit. It should be at least one, result of squashing together all the previous ones.

njeudy · 2018-01-11T17:14:15Z

@pedrobaeza yes, look at [8.0][website_anchor_smooth_scroll] Smooth scrolling on anchors. you have transbot file squashed in. and idem for [MIG] website_anchor_smooth_scroll: Migration to 10.0

I can do differently but as we only want to have clean history I think it can be better like this ..

pedrobaeza · 2018-01-11T17:18:08Z

Yeah, but we shouldn't clean that way. Translations should be kept apart and with "OCA Transbot" identity, or all the lines of diff will be attributed to the person who makes the translation on the future contributor stats, which is not correct.

njeudy · 2018-01-11T17:26:43Z

ok I will rebase with transbot commit ...

njeudy · 2018-01-11T19:07:07Z

@pedrobaeza ok done :) clean rebase.

pedrobaeza · 2018-01-12T15:21:15Z

Trying on runbot the demo page "Anchor smooth scroll demo", images that were available in previous version aren't now. Can you please change then for proper test?

pedrobaeza · 2018-01-12T15:27:30Z

I have cleaned a bit more the commits, because the first 2 are from the same author, and the renaming plus installable=False are not of significance.

njeudy · 2018-01-13T13:51:41Z

@pedrobaeza done !

njeudy · 2018-01-23T20:34:25Z

@pedrobaeza can you explain me why runbot is red ? find no errors ..

njeudy · 2018-01-24T20:17:35Z

@simahawk @yajo could you review this one in same times ? :)

simahawk · 2018-01-25T08:12:39Z

@njeudy can you review your PR 1st? README and manifest need the usual fixes

njeudy · 2018-01-25T08:33:55Z

@simahawk found wrong website url in manifest but did not catch problems in README.

Sorry if I mess something ..

simahawk · 2018-01-25T09:24:58Z

+    if (event.target.hash != "") {
+      var target = $(event.target.hash);
+      return $('html, body')
+      .stop()


not blocking but these lines should be indented under the return statement

Ok done, you're right :)

yajo · 2018-01-26T11:18:29Z

+Configuration
+=============
+
+No configuration needed


You can drop the configuration section then

yajo · 2018-01-26T11:18:44Z

+Funders
+-------
+
+None


Again, empty sections can be dropped

yajo · 2018-01-26T11:19:37Z

 * License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl). */
+odoo.define('website_anchor_smooth_scroll.website_anchor_smooth_scroll', function (require) {

+require('web.dom_ready');


Do not put anything before "use strict" or it has no meaning. Besides, this call must appear above 1st dom modifications, not here.

yajo · 2018-01-26T11:24:21Z

 }

-(function ($) {
+$(document).ready(function () {


Just put the require('web.dom_ready'); line above this and remove this callback.

Get it for free for all of your `<a href="#whatever">Move me</a>` elements!

- [FIX] remove en.po that was erroneously created by transbot

njeudy · 2018-01-26T12:56:13Z

@yajo ok thank for the review, changes done.

yajo · 2018-01-26T13:19:48Z

    // Apply to all links that start with `#`
-    $("a[href^='#']").live("click", website_anchor_smooth_scroll);
-})(jQuery);
+    $("a[href^='#']:not([href=#])").on("click", website_anchor_smooth_scroll);


This function is being declared but never executed, just execute this line, without wrapping it in a function please 😊

njeudy · 2018-01-27T18:42:42Z

@yajo ok done, thanks a lot for review it helps me a lot .. and give really clean code :)

yajo

Thanks to you @njeudy. BTW on successive reviews it's good to add one !fixup commit on top of the last one instead of ammending. It helps the reviewier to concentrate on the new diff rather than reviewing again all the change. Then, after the final OK is given, you squash it and we merge it.

Well, in any case it's OK now. Merging.

njeudy mentioned this pull request Jan 11, 2018

Migration to version 11.0 #388

Closed

38 tasks

pedrobaeza added this to the 11.0 milestone Jan 11, 2018

pedrobaeza added the needs review label Jan 11, 2018

njeudy force-pushed the 11-mig-website_anchor_smooth_scroll branch from 116f9f2 to 430981e Compare January 11, 2018 16:54

njeudy commented Jan 11, 2018

View reviewed changes

njeudy force-pushed the 11-mig-website_anchor_smooth_scroll branch 2 times, most recently from 40c25b6 to c9d8fde Compare January 11, 2018 17:02

njeudy force-pushed the 11-mig-website_anchor_smooth_scroll branch from c9d8fde to d09f5e2 Compare January 11, 2018 17:06

njeudy force-pushed the 11-mig-website_anchor_smooth_scroll branch from d09f5e2 to da48a28 Compare January 11, 2018 19:06

njeudy changed the title ~~WIP : [MIG] website_anchor_smooth_scroll: Migration to 11.0~~ [MIG] website_anchor_smooth_scroll: Migration to 11.0 Jan 11, 2018

pedrobaeza force-pushed the 11-mig-website_anchor_smooth_scroll branch 3 times, most recently from e0d83cc to f95321d Compare January 12, 2018 15:26

njeudy force-pushed the 11-mig-website_anchor_smooth_scroll branch 2 times, most recently from 1343af8 to 1d618cb Compare January 12, 2018 20:50

njeudy force-pushed the 11-mig-website_anchor_smooth_scroll branch from 1d618cb to d9c8592 Compare January 14, 2018 22:34

njeudy force-pushed the 11-mig-website_anchor_smooth_scroll branch from d9c8592 to 87b6bd8 Compare January 15, 2018 15:33

njeudy force-pushed the 11-mig-website_anchor_smooth_scroll branch from 87b6bd8 to ec5df2d Compare January 23, 2018 15:17

njeudy force-pushed the 11-mig-website_anchor_smooth_scroll branch 2 times, most recently from 5ddbd18 to ac69f7a Compare January 25, 2018 08:32

simahawk approved these changes Jan 25, 2018

View reviewed changes

njeudy force-pushed the 11-mig-website_anchor_smooth_scroll branch from ac69f7a to 846a2e7 Compare January 25, 2018 10:42

yajo requested changes Jan 26, 2018

View reviewed changes

yajo and others added 4 commits January 26, 2018 12:35

[8.0][website_anchor_smooth_scroll] Smooth scrolling on anchors.

9fe7686

Get it for free for all of your `<a href="#whatever">Move me</a>` elements!

[IMP] website_anchor_smooth_scroll: Add author

2ddc6f2

OCA Transbot updated translations from Transifex

ce81f88

- [FIX] remove en.po that was erroneously created by transbot

[MIG] website_anchor_smooth_scroll: Migration to 10.0

b271102

njeudy force-pushed the 11-mig-website_anchor_smooth_scroll branch 2 times, most recently from 6ca1160 to 3a5afe5 Compare January 26, 2018 12:54

yajo requested changes Jan 26, 2018

View reviewed changes

[MIG] website_anchor_smooth_scroll: Migration to 11.0

0a665f8

njeudy force-pushed the 11-mig-website_anchor_smooth_scroll branch from 3a5afe5 to 0a665f8 Compare January 27, 2018 17:22

yajo approved these changes Jan 29, 2018

View reviewed changes

yajo merged commit efdbd02 into OCA:11.0 Jan 29, 2018

		@@ -0,0 +1,28 @@
		# -- coding: utf-8 --
		# © 2016 Antiun Ingeniería S.L. - Jairo Llopis

+              Funders
+              -------
+              None

Uh oh!

Conversation

njeudy commented Jan 11, 2018

Uh oh!

oca-clabot commented Jan 11, 2018

Uh oh!

pedrobaeza commented Jan 11, 2018

Uh oh!

njeudy commented Jan 11, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pedrobaeza commented Jan 11, 2018

Uh oh!

njeudy commented Jan 11, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pedrobaeza commented Jan 11, 2018

Uh oh!

njeudy commented Jan 11, 2018

Uh oh!

pedrobaeza commented Jan 11, 2018

Uh oh!

njeudy commented Jan 11, 2018

Uh oh!

njeudy commented Jan 11, 2018

Uh oh!

pedrobaeza commented Jan 12, 2018

Uh oh!

pedrobaeza commented Jan 12, 2018

Uh oh!

njeudy commented Jan 13, 2018

Uh oh!

njeudy commented Jan 23, 2018

Uh oh!

njeudy commented Jan 24, 2018

Uh oh!

simahawk commented Jan 25, 2018

Uh oh!

njeudy commented Jan 25, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

njeudy commented Jan 26, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

njeudy commented Jan 27, 2018

Uh oh!

yajo left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

njeudy commented Jan 11, 2018 •

edited

Loading