Changes based on code review.

src/clients/ntp-client_proposal.rb

@@ -198,8 +198,8 @@ def MakeProposal
       # Once read or proposed any config we consider it as read (bnc#427712)
       NtpClient.config_has_been_read = true
 
-      ntp_items = Builtins.add(ntp_items, "")
-      Builtins.y2milestone("ntp_items :%1", ntp_items)
+      ntp_items << ""
+      log.info "ntp_items :#{ntp_items}"
       UI.ChangeWidget(Id(:ntp_address), :Items, ntp_items)
 
       nil
@@ -473,33 +473,31 @@ def add_or_install_required_package
       end
     end
 
-    # Configured ntp servers Yast::Term items with the ntp address as the ID
-    # and as the label
+    # Configured ntp servers Yast::Term items with the ntp address ID and label
     #
     # @return [Yast::Term] ntp address table Item
     def configured_ntp_items
       NtpClient.GetUsedNtpServers.map { |s| Item(Id(s), s) }
     end
 
-    # Public list of ntp servers Yast::Term items with the ntp address as the
-    # ID and as the label
+    # Public list of ntp servers Yast::Term items with the ntp address ID and
+    # label
     #
     # @return [Array<Yast::Term>] ntp address Item
     def timezone_ntp_items
       timezone_country = Timezone.GetCountryForTimezone(Timezone.timezone)
       NtpClient.GetNtpServersByCountry(timezone_country, true)
     end
 
-    # List of dhcp ntp servers Yast::Term items with the ntp address as the
-    # ID and as the label
+    # List of dhcp ntp servers Yast::Term items with the ntp address ID and
+    # label
     #
     # @return [Array<Yast::Term>] ntp address table Item
     def dhcp_ntp_items
       NtpClient.dhcp_ntp_servers.map { |s| Item(Id(s), s) }
     end
 
-    # List of ntp servers Yast::Term items with the ntp address as the ID and
-    # label
+    # List of ntp servers Yast::Term items with the ntp address ID and label
     #
     # @return [Array<Yast::Term>] ntp address table Item
     def fallback_ntp_items

src/lib/y2ntp_client/dialog/add_pool.rb

@@ -37,6 +37,7 @@ def initialize(address_widget, pool_type)
 
       # @macro seeDialog
       def title
+        # TRANSLATORS: dialog title
         _("Local ntp servers discovered")
       end
 
@@ -74,9 +75,12 @@ def ok_button
         PushButton(Id(:next), Opt(:default), ok_button_label)
       end
 
+      def available_pools
+        { local:  Widgets::LocalList, public: Widgets::PublicList }
+      end
+
       def pool_for(type)
-        { local:  Widgets::LocalList,
-          public: Widgets::PublicList }.fetch(type, Widgets::LocalList).new(@address_widget.value)
+        available_pools.fetch(type, Widgets::LocalList).new(@address_widget.value)
       end
 
       def min_height

src/lib/y2ntp_client/widgets/main_widgets.rb

@@ -24,13 +24,14 @@ def label
       end
 
       def help
-        # TRANSLATORS: 'man 8 netconfig' is a command, do not translate that
-        _(
-          "<p>The NTP configuration may be provided by the local network over DHCP. " \
-          "<b>Configuration Source</b> can simply enable or disable using that configuration. " \
-          "In cases where there may be multiple DHCP sources, it can prioritize them: " \
-          "see 'man 8 netconfig'.</p>"
-        )
+        "<p>" +
+          # TRANSLATORS: 'man 8 netconfig' is a command, do not translate that
+          _(
+            "<p>The NTP configuration may be provided by the local network over DHCP. " \
+            "<b>Configuration Source</b> can simply enable or disable using that configuration. " \
+            "In cases where there may be multiple DHCP sources, it can prioritize them: " \
+            "see 'man 8 netconfig'.</p>"
+          ) + "</p>"
       end
 
       def opt

src/lib/y2ntp_client/widgets/pool_widgets.rb

@@ -21,6 +21,7 @@ def initialize(initial_value)
 
       # @macro seeAbstractWidget
       def label
+        # TRANSLATORS: widget label
         _("A&ddress")
       end
 
@@ -52,6 +53,7 @@ def initialize(address_widget)
       end
 
       def label
+        # TRANSLATORS: widget label
         _("Test")
       end
 
@@ -70,6 +72,7 @@ def initialize(options)
       end
 
       def label
+        # TRANSLATORS: widget label
         _("Quick Initial Sync")
       end
 
@@ -89,8 +92,7 @@ def help
         "<p>" +
           _("<b>Quick Initial Sync</b> specifies whether the 'iburst' option is used. This " \
           "option sends 4 poll requests in 2 second intervals during the initialization. It is " \
-          "useful for a quick synchronization during the start of the machine.") +
-          "</p>"
+          "useful for a quick synchronization during the start of the machine.") + "</p>"
       end
     end
 
@@ -102,6 +104,7 @@ def initialize(options)
       end
 
       def label
+        # TRANSLATORS: widget label
         _("Start Offline")
       end
 
@@ -118,10 +121,11 @@ def store
       end
 
       def help
-        _("<p><b>Start Offline</b> specifies whether the 'offline' option is used. This option " \
-          "skips this server during the start-up. It is useful for a machine which starts " \
-          "without the network, because it speeds up the boot, and synchronizes when the machine " \
-          "gets connected to the network.</p>")
+        "<p>" +
+          _("<b>Start Offline</b> specifies whether the 'offline' option is used. This option " \
+            "skips this server during the start-up. It is useful for a machine which starts " \
+            "without the network, because it speeds up the boot, and synchronizes when the " \
+            "machine gets connected to the network.") + "</p>"
       end
     end
 
@@ -134,6 +138,7 @@ def initialize(address_widget)
 
       # @macro seeAbstractWidget
       def label
+        # TRANSLATORS: widget label
         _("Select")
       end
 
@@ -183,6 +188,7 @@ class LocalList < CWM::SelectionBox
       # @param address [String] current ntp pool address
       def initialize(address)
         textdomain "ntp-client"
+
         @address = address
         @servers = []
       end
@@ -200,6 +206,7 @@ def opt
 
       # @macro seeAbstractWidget
       def label
+        # TRANSLATORS: widget label
         _("Syncronization server")
       end
 
@@ -246,12 +253,14 @@ class PublicList < CWM::CustomWidget
       # @param address [String] current ntp pool address
       def initialize(address)
         textdomain "ntp-client"
+
         @country_pools = CountryPools.new
         @country = Country.new(country_for(address), @country_pools)
       end
 
       # @macro seeAbstractWidget
       def label
+        # TRANSLATORS: widget label
         _("Public Servers")
       end
 
@@ -271,7 +280,7 @@ def value
 
       # @macro seeAbstractWidget
       def help
-        _("<p>List of public ntp servers filtered by Country.")
+        _("<p>List of public ntp servers filtered by country.</p>")
       end
 
     private
@@ -303,6 +312,7 @@ def opt
 
       # @macro seeAbstractWidget
       def label
+        # TRANSLATORS: widget label
         _("Country")
       end
 
@@ -342,6 +352,7 @@ def opt
 
       # macro seeAbstractWidget
       def label
+        # TRANSLATORS: widget label
         _("Ntp Servers")
       end