ChangeLogTag:Tue Mar 28 15:58:36 2000 Carlos O'Ryan <coryan@uci.edu>

26 files changed, 318 insertions, 57 deletions

diff --git a/TAO/ChangeLogs/ChangeLog-02a b/TAO/ChangeLogs/ChangeLog-02a
index ce695eff59d..f5da51ed029 100644
--- a/TAO/ChangeLogs/ChangeLog-02a
+++ b/TAO/ChangeLogs/ChangeLog-02a
@@ -1,3 +1,32 @@
+Tue Mar 28 15:58:36 2000  Carlos O'Ryan  <coryan@uci.edu>
+
+	* orbsvcs/examples/Makefile:
+	* orbsvcs/examples/Notify/Makefile:
+	* orbsvcs/examples/Notify/Filter/Filter.cpp:
+	* orbsvcs/examples/Notify/Filter/Makefile:
+	* orbsvcs/examples/Notify/Subscribe/Makefile:
+	* orbsvcs/orbsvcs/Notify/Notify_EventChannel_i.h:
+	* orbsvcs/orbsvcs/Notify/Notify_Proxy_T.h:
+	* orbsvcs/orbsvcs/Notify/Notify_PushSupplier.h:
+	* orbsvcs/orbsvcs/Notify/Notify_QoSAdmin_i.cpp:
+	* orbsvcs/orbsvcs/Notify/Notify_QoSAdmin_i.h:
+	* orbsvcs/orbsvcs/Notify/Notify_Resource_Manager.cpp:
+	* orbsvcs/orbsvcs/Notify/Notify_Resource_Manager.h:
+	* orbsvcs/orbsvcs/Notify/Notify_SequenceProxyPushConsumer_i.cpp:
+	* orbsvcs/orbsvcs/Notify/Notify_SequenceProxyPushSupplier_i.cpp:
+	* orbsvcs/orbsvcs/Notify/Notify_SequenceProxyPushSupplier_i.h:
+	* orbsvcs/orbsvcs/Notify/Notify_StructuredProxyPushConsumer_i.cpp:
+	* orbsvcs/orbsvcs/Notify/Notify_StructuredProxyPushSupplier_i.cpp:
+	* orbsvcs/orbsvcs/Notify/Notify_StructuredPushConsumer.cpp:
+	* orbsvcs/orbsvcs/Notify/Notify_StructuredPushConsumer.h:
+	* orbsvcs/orbsvcs/Notify/Notify_StructuredPushSupplier.cpp:
+	* orbsvcs/orbsvcs/Notify/Notify_StructuredPushSupplier.h:
+	* orbsvcs/orbsvcs/Notify/Notify_SupplierAdmin_i.cpp:
+	* orbsvcs/orbsvcs/Notify/Notify_SupplierAdmin_i.h:
+	* orbsvcs/orbsvcs/Notify/Notify_Types.cpp:
+	* orbsvcs/orbsvcs/Notify/Notify_Types.h:
+	  Another bunch of @@ comments for Pradeep.
+
 Tue Mar 28 14:40:54 2000  Carlos O'Ryan  <coryan@uci.edu>
 
 	* tao/GIOP_Message_Accept_State.h:
diff --git a/TAO/orbsvcs/examples/Makefile b/TAO/orbsvcs/examples/Makefile
index d4299c7603b..0d1d9bc95a2 100644
--- a/TAO/orbsvcs/examples/Makefile
+++ b/TAO/orbsvcs/examples/Makefile
@@ -9,7 +9,8 @@
 #----------------------------------------------------------------------------
 
 DIRS  =	CosEC \
-	RtEC
+	RtEC \
+	Notify
 
 ifndef TAO_ROOT
   TAO_ROOT = $(ACE_ROOT)/TAO
diff --git a/TAO/orbsvcs/examples/Notify/Filter/Filter.cpp b/TAO/orbsvcs/examples/Notify/Filter/Filter.cpp
index 93634cae215..5a1e155957f 100644
--- a/TAO/orbsvcs/examples/Notify/Filter/Filter.cpp
+++ b/TAO/orbsvcs/examples/Notify/Filter/Filter.cpp
@@ -3,12 +3,17 @@
 #include "orbsvcs/orbsvcs/Notify/Notify_StructuredPushSupplier.h"
 #include "Filter.h"
 
+// @@ Pradeep: local #includes should go first.
+
 #define NOTIFY_FACTORY_NAME "NotifyEventChannelFactory"
 #define NAMING_SERVICE_NAME "NameService"
 #define CA_FILTER "threshold < 20"
 #define SA_FILTER "threshold > 10"
+// @@ Pradeep: it is 'GRAMMAR' not 'GRAMMER'
 #define TCL_GRAMMER "TCL"
 
+// @@ Pradeep: please don't put classes in the .cpp file...
+
 class PushConsumer : public TAO_Notify_StructuredPushConsumer
 {
 private:
@@ -33,6 +38,10 @@ public:
 
     notification.remainder_of_body >>= val;
 
+    // @@ Pradeep: for your tests try to make sure that you count the
+    // number of expected and sent events to verify that things work
+    // correctly in an automatic way...
+
     ACE_DEBUG ((LM_DEBUG,
                 "%s received event %d\n", myname_.fast_rep (),
                 val));
diff --git a/TAO/orbsvcs/examples/Notify/Filter/Makefile b/TAO/orbsvcs/examples/Notify/Filter/Makefile
index f3ecaf0753a..34c9fa42674 100644
--- a/TAO/orbsvcs/examples/Notify/Filter/Makefile
+++ b/TAO/orbsvcs/examples/Notify/Filter/Makefile
@@ -12,7 +12,7 @@ ifndef TAO_ROOT
   TAO_ROOT = $(ACE_ROOT)/TAO
 endif
 
-LDLIBS = -lTAO_CosNaming -lTAO
+LDLIBS = -lTAO_Notify -lTAO_CosTrading -lTAO_CosNaming -TAO_Svc_Utils -lTAO
 
 LSRC = Filter.cpp main.cpp
 
diff --git a/TAO/orbsvcs/examples/Notify/Makefile b/TAO/orbsvcs/examples/Notify/Makefile
new file mode 100644
index 00000000000..453e14a22fd
--- /dev/null
+++ b/TAO/orbsvcs/examples/Notify/Makefile
@@ -0,0 +1,27 @@
+#----------------------------------------------------------------------------
+#
+# $Id$
+#
+#----------------------------------------------------------------------------
+
+#----------------------------------------------------------------------------
+#	Local macros
+#----------------------------------------------------------------------------
+
+DIRS  =	Subscribe \
+	Filter
+
+ifndef TAO_ROOT
+  TAO_ROOT = $(ACE_ROOT)/TAO
+endif
+
+#----------------------------------------------------------------------------
+#	Include macros and targets
+#----------------------------------------------------------------------------
+
+include	$(ACE_ROOT)/include/makeinclude/wrapper_macros.GNU
+include	$(ACE_ROOT)/include/makeinclude/macros.GNU
+include $(TAO_ROOT)/rules.tao.GNU
+include	$(ACE_ROOT)/include/makeinclude/rules.common.GNU
+include	$(ACE_ROOT)/include/makeinclude/rules.nested.GNU
+include	$(ACE_ROOT)/include/makeinclude/rules.nolocal.GNU
diff --git a/TAO/orbsvcs/examples/Notify/Subscribe/Makefile b/TAO/orbsvcs/examples/Notify/Subscribe/Makefile
index bdaa357f200..c9f1b3d6a87 100644
--- a/TAO/orbsvcs/examples/Notify/Subscribe/Makefile
+++ b/TAO/orbsvcs/examples/Notify/Subscribe/Makefile
@@ -12,7 +12,7 @@ ifndef TAO_ROOT
   TAO_ROOT = $(ACE_ROOT)/TAO
 endif
 
-LDLIBS = -lTAO_CosNaming -lTAO
+LDLIBS = -lTAO_Notify -lTAO_CosTrading -lTAO_CosNaming -TAO_Svc_Utils -lTAO
 
 LSRC = Subscribe.cpp main.cpp
 
diff --git a/TAO/orbsvcs/orbsvcs/Notify/Notify_EventChannel_i.h b/TAO/orbsvcs/orbsvcs/Notify/Notify_EventChannel_i.h
index a5adeae7164..ccc81715666 100644
--- a/TAO/orbsvcs/orbsvcs/Notify/Notify_EventChannel_i.h
+++ b/TAO/orbsvcs/orbsvcs/Notify/Notify_EventChannel_i.h
@@ -165,7 +165,7 @@ virtual CosNotification::QoSProperties * get_qos (
     CORBA::SystemException
   ));
 
-virtual void set_qos (
+  virtual void set_qos (
     const CosNotification::QoSProperties & qos,
     CORBA::Environment &ACE_TRY_ENV
   )
@@ -174,7 +174,7 @@ virtual void set_qos (
     CosNotification::UnsupportedQoS
   ));
 
-virtual void validate_qos (
+  virtual void validate_qos (
     const CosNotification::QoSProperties & required_qos,
     CosNotification::NamedPropertyRangeSeq_out available_qos,
     CORBA::Environment &ACE_TRY_ENV
@@ -184,14 +184,14 @@ virtual void validate_qos (
     CosNotification::UnsupportedQoS
   ));
 
-virtual CosNotification::AdminProperties * get_admin (
+  virtual CosNotification::AdminProperties * get_admin (
     CORBA::Environment &ACE_TRY_ENV
   )
   ACE_THROW_SPEC ((
     CORBA::SystemException
   ));
 
-virtual void set_admin (
+  virtual void set_admin (
     const CosNotification::AdminProperties & admin,
     CORBA::Environment &ACE_TRY_ENV
   )
@@ -200,30 +200,30 @@ virtual void set_admin (
     CosNotification::UnsupportedAdmin
   ));
 
-virtual CosEventChannelAdmin::ConsumerAdmin_ptr for_consumers (
+  virtual CosEventChannelAdmin::ConsumerAdmin_ptr for_consumers (
     CORBA::Environment &ACE_TRY_ENV
   )
   ACE_THROW_SPEC ((
     CORBA::SystemException
   ));
 
-virtual CosEventChannelAdmin::SupplierAdmin_ptr for_suppliers (
+  virtual CosEventChannelAdmin::SupplierAdmin_ptr for_suppliers (
     CORBA::Environment &ACE_TRY_ENV
   )
   ACE_THROW_SPEC ((
     CORBA::SystemException
   ));
 
-virtual void destroy (
+  virtual void destroy (
     CORBA::Environment &ACE_TRY_ENV
   )
   ACE_THROW_SPEC ((
                    CORBA::SystemException
   ));
 
- protected:
-// = Helper Methods
- CosNotifyFilter::FilterFactory_ptr create_default_filter_factory_i (CORBA::Environment& ACE_TRY_ENV);
+protected:
+  // = Helper Methods
+  CosNotifyFilter::FilterFactory_ptr create_default_filter_factory_i (CORBA::Environment& ACE_TRY_ENV);
  // Create the default filter factory.
 
  void cleanup_i (CORBA::Environment &ACE_TRY_ENV = TAO_default_environment ());
diff --git a/TAO/orbsvcs/orbsvcs/Notify/Notify_Proxy_T.h b/TAO/orbsvcs/orbsvcs/Notify/Notify_Proxy_T.h
index beb2508f6e3..409b589fdcb 100644
--- a/TAO/orbsvcs/orbsvcs/Notify/Notify_Proxy_T.h
+++ b/TAO/orbsvcs/orbsvcs/Notify/Notify_Proxy_T.h
@@ -36,6 +36,10 @@ class TAO_Notify_Event_Manager;
 #pragma warning(disable:4250)
 #endif /* _MSC_VER */
 
+// @@ Pradeep: this is cool, deriving from the template type is really
+// neat.  I bet it is going to break something like 90% of the
+// compilers, but they deserve it! ;-) ;-)
+
 template <class SERVANT_TYPE>
 class TAO_Notify_Export TAO_Notify_Proxy : public SERVANT_TYPE, public TAO_Notify_Update_Listener
 {
diff --git a/TAO/orbsvcs/orbsvcs/Notify/Notify_PushSupplier.h b/TAO/orbsvcs/orbsvcs/Notify/Notify_PushSupplier.h
index 161abf25d67..a0b257f09ba 100644
--- a/TAO/orbsvcs/orbsvcs/Notify/Notify_PushSupplier.h
+++ b/TAO/orbsvcs/orbsvcs/Notify/Notify_PushSupplier.h
@@ -14,7 +14,7 @@
 
 class TAO_Notify_PushSupplier : public POA_CosNotifyComm::PushSupplier, public PortableServer::RefCountServantBase
 {
- public:
+public:
     // = Initialization and Termination code
   TAO_Notify_PushSupplier (void);
   // Constructor.
diff --git a/TAO/orbsvcs/orbsvcs/Notify/Notify_QoSAdmin_i.cpp b/TAO/orbsvcs/orbsvcs/Notify/Notify_QoSAdmin_i.cpp
index e2aaa7d7454..6b8be7189e8 100644
--- a/TAO/orbsvcs/orbsvcs/Notify/Notify_QoSAdmin_i.cpp
+++ b/TAO/orbsvcs/orbsvcs/Notify/Notify_QoSAdmin_i.cpp
@@ -4,48 +4,50 @@
 
 // Implementation skeleton constructor
 TAO_Notify_QoSAdmin_i::TAO_Notify_QoSAdmin_i (void)
-  {
-  }
+{
+}
 
 // Implementation skeleton destructor
 TAO_Notify_QoSAdmin_i::~TAO_Notify_QoSAdmin_i (void)
-  {
-  }
+{
+}
 
 CosNotification::QoSProperties * TAO_Notify_QoSAdmin_i::get_qos (
-                                                                 CORBA::Environment & ///ACE_TRY_ENV
-                                                                 )
+    CORBA::Environment & ///ACE_TRY_ENV
+  )
   ACE_THROW_SPEC ((
-                   CORBA::SystemException
-                   ))
+       CORBA::SystemException
+     ))
 {
   //Add your implementation here
   return 0;
 }
 
-void TAO_Notify_QoSAdmin_i::set_qos (
-                                     const CosNotification::QoSProperties & /*qos*/,
-                                     CORBA::Environment & //ACE_TRY_ENV
+void
+TAO_Notify_QoSAdmin_i::set_qos (
+    const CosNotification::QoSProperties & /*qos*/,
+    CORBA::Environment & //ACE_TRY_ENV
   )
   ACE_THROW_SPEC ((
     CORBA::SystemException,
     CosNotification::UnsupportedQoS
   ))
-  {
-    //Add your implementation here
-    return;
-  }
+{
+  //Add your implementation here
+  return;
+}
 
-void TAO_Notify_QoSAdmin_i::validate_qos (
-                                          const CosNotification::QoSProperties & /*required_qos*/,
-                                          CosNotification::NamedPropertyRangeSeq_out /*available_qos*/,
-                                          CORBA::Environment & //ACE_TRY_ENV
+void
+TAO_Notify_QoSAdmin_i::validate_qos (
+    const CosNotification::QoSProperties & /*required_qos*/,
+    CosNotification::NamedPropertyRangeSeq_out /*available_qos*/,
+    CORBA::Environment & //ACE_TRY_ENV
   )
   ACE_THROW_SPEC ((
     CORBA::SystemException,
     CosNotification::UnsupportedQoS
   ))
-  {
-    //Add your implementation here
-    return;
-  }
+{
+  //Add your implementation here
+  return;
+}
diff --git a/TAO/orbsvcs/orbsvcs/Notify/Notify_QoSAdmin_i.h b/TAO/orbsvcs/orbsvcs/Notify/Notify_QoSAdmin_i.h
index b4388336c73..68bb71db2ba 100644
--- a/TAO/orbsvcs/orbsvcs/Notify/Notify_QoSAdmin_i.h
+++ b/TAO/orbsvcs/orbsvcs/Notify/Notify_QoSAdmin_i.h
@@ -66,5 +66,6 @@ public:
     CosNotification::UnsupportedQoS
   ));
 };
+
 #include "ace/post.h"
 #endif /* TAO_NOTIFY_QOSADMIN_I_H */
diff --git a/TAO/orbsvcs/orbsvcs/Notify/Notify_Resource_Manager.cpp b/TAO/orbsvcs/orbsvcs/Notify/Notify_Resource_Manager.cpp
index 5c5f1374812..9cceb543395 100644
--- a/TAO/orbsvcs/orbsvcs/Notify/Notify_Resource_Manager.cpp
+++ b/TAO/orbsvcs/orbsvcs/Notify/Notify_Resource_Manager.cpp
@@ -12,14 +12,18 @@
 #include "Notify_SequenceProxyPushConsumer_i.h"
 #include "Notify_ProxyPushConsumer_i.h"
 
+// @@ Pradeep: what happened to the RCSID macro?
+
 TAO_Notify_Resource_Manager::TAO_Notify_Resource_Manager (PortableServer::POA_ptr default_POA)
   :default_POA_ (PortableServer::POA::_duplicate (default_POA))
 {
+  // @@ Pradeep: this is a perfectly useless comment, don't you think?
   // No-Op.
 }
 
 TAO_Notify_Resource_Manager::~TAO_Notify_Resource_Manager ()
 {
+  // @@ Pradeep: this is a perfectly useless comment, don't you think?
   // No-Op.
 }
 
@@ -185,6 +189,12 @@ TAO_Notify_Resource_Manager::create_proxy_pushsupplier_POA (PortableServer::POA_
 PortableServer::POA_ptr
 TAO_Notify_Resource_Manager::create_generic_childPOA_i (PortableServer::POA_ptr poa, CORBA::Environment &ACE_TRY_ENV)
 {
+  // @@ Pradeep: if the Notification service is ever going to be
+  // persistent or fault tolerant you may need to create this stuff
+  // with the persistent policy too, probably you can handle that
+  // using a different 'Resource_Manager' that overrides this
+  // method. Just a thought...
+
   // Create a UNIQUE_ID and USER_ID policy because we want the POA
   // to detect duplicates for us.
   PortableServer::IdUniquenessPolicy_var idpolicy =
@@ -209,6 +219,7 @@ TAO_Notify_Resource_Manager::create_generic_childPOA_i (PortableServer::POA_ptr
   policy_list [1] =
     PortableServer::IdAssignmentPolicy::_duplicate (assignpolicy.in ());
 
+  // @@ Pradeep: is it possible to use a more meaningful name?
   char child_poa_name[BUFSIZ];
 
   CORBA::Long poa_id = this->poa_ids_.get ();
@@ -220,6 +231,8 @@ TAO_Notify_Resource_Manager::create_generic_childPOA_i (PortableServer::POA_ptr
                                                      manager,
                                                      policy_list,
                                                      ACE_TRY_ENV);
+  // @@ Pradeep: these guys take an ACE_TRY_ENV argument, and can
+  // raise exceptions.  WOuld you believe that?
   idpolicy->destroy ();
   assignpolicy->destroy ();
 
@@ -247,6 +260,10 @@ TAO_Notify_Resource_Manager::long_to_ObjectId (const CORBA::Long id)
   // Copy the contents
   ACE_OS::memcpy (buffer, (char*)&id, buffer_size);
 
+  // @@ Pradeep: TAO guarantees that Long is 4 bytes wide, but the
+  // standard only guarantees that it is at least 4 bytes wide. You
+  // may want to think about that....
+
   // Create and return a new ID
   PortableServer::ObjectId *obj_id = 0;
   ACE_NEW_RETURN (obj_id,
diff --git a/TAO/orbsvcs/orbsvcs/Notify/Notify_Resource_Manager.h b/TAO/orbsvcs/orbsvcs/Notify/Notify_Resource_Manager.h
index 8dc5cdf6caa..e43ccb06ea4 100644
--- a/TAO/orbsvcs/orbsvcs/Notify/Notify_Resource_Manager.h
+++ b/TAO/orbsvcs/orbsvcs/Notify/Notify_Resource_Manager.h
@@ -18,6 +18,8 @@
 #define TAO_NOTIFY_RESOURCE_MANAGER
 #include "ace/pre.h"
 
+// @@ Pradeep: do not forget the #pragma once.
+
 #include "Notify_ID_Pool_T.h"
 #include "orbsvcs/CosNotifyChannelAdminS.h"
 #include "tao/POA.h"
@@ -35,6 +37,7 @@ class TAO_Notify_SequenceProxyPushConsumer_i;
 class TAO_Notify_ProxyPushConsumer_i;
 class TAO_Notify_Event_Manager;
 
+// @@ Pradeep: do not forget the TAO_****_Export
 class TAO_Notify_Resource_Manager
 {
   // = TITLE
@@ -43,7 +46,18 @@ class TAO_Notify_Resource_Manager
   // = DESCRIPTION
   //   This is class to control all the resources needed by all the other
   //   classes.
-  //   Later:The class is also a Service Object that will configure the
+  //
+  //   @@ Pradeep: this is a class a factory? Or is it a manager for
+  //   resource created by a factory?  The interfaces should be quite
+  //   different, and it is hard to turn a manager into a Service
+  //   Object (but factories are much easier).  If you check the
+  //   design of the RTEC and the new CosEC you will notice that the
+  //   Event_Channel class plays the manager role and the factory is a
+  //   separate entity.  Your idea of separating the manager from the
+  //   Event_Channel is probably the "Right Thing" but you should also
+  //   decouple the factory and manager roles.
+  //
+  //   Later: The class is also a Service Object that will configure the
   //   service on startup.
 
  public:
@@ -57,6 +71,8 @@ class TAO_Notify_Resource_Manager
   // = Factory method
   static TAO_Notify_Resource_Manager* create (PortableServer::POA_ptr default_POA, CORBA::Environment &ACE_TRY_ENV);
   // Factory method to create the Resource Factory Manager.
+  // @@ Pradeep: how is that possible?  I still have to know the class
+  // to create it, right?
   // This allows other specializations of the Resource Factory to be created
   // via a common interface.
 
diff --git a/TAO/orbsvcs/orbsvcs/Notify/Notify_SequenceProxyPushConsumer_i.cpp b/TAO/orbsvcs/orbsvcs/Notify/Notify_SequenceProxyPushConsumer_i.cpp
index d8d6e48ef13..ce3ff47956f 100644
--- a/TAO/orbsvcs/orbsvcs/Notify/Notify_SequenceProxyPushConsumer_i.cpp
+++ b/TAO/orbsvcs/orbsvcs/Notify/Notify_SequenceProxyPushConsumer_i.cpp
@@ -31,6 +31,10 @@ TAO_Notify_SequenceProxyPushConsumer_i::connect_sequence_push_supplier (CosNotif
                    CosEventChannelAdmin::AlreadyConnected
                    ))
 {
+  // @@ Pradeep: here is another example on code that is not thread
+  // safe, i know you are post-poning the thread safety issues, but it
+  // is not that easy!
+
   if (this->is_connected_ == 1)
     ACE_THROW (CosEventChannelAdmin::AlreadyConnected ());
   else
diff --git a/TAO/orbsvcs/orbsvcs/Notify/Notify_SequenceProxyPushSupplier_i.cpp b/TAO/orbsvcs/orbsvcs/Notify/Notify_SequenceProxyPushSupplier_i.cpp
index 5f9dde02f8d..3a5dda4d424 100644
--- a/TAO/orbsvcs/orbsvcs/Notify/Notify_SequenceProxyPushSupplier_i.cpp
+++ b/TAO/orbsvcs/orbsvcs/Notify/Notify_SequenceProxyPushSupplier_i.cpp
@@ -18,6 +18,7 @@ TAO_Notify_SequenceProxyPushSupplier_i::~TAO_Notify_SequenceProxyPushSupplier_i
 void
 TAO_Notify_SequenceProxyPushSupplier_i::cleanup_i (CORBA::Environment &ACE_TRY_ENV)
 {
+  // @@ Pradeep: maybe a typedef would simplify this...
   TAO_Notify_ProxySupplier<POA_CosNotifyChannelAdmin::SequenceProxyPushSupplier>::cleanup_i (ACE_TRY_ENV);
 
   this->push_consumer_ = CosNotifyComm::SequencePushConsumer::_nil ();
@@ -98,11 +99,13 @@ TAO_Notify_SequenceProxyPushSupplier_i::disconnect_sequence_push_supplier(
 }
 
 #if defined (ACE_HAS_EXPLICIT_TEMPLATE_INSTANTIATION)
+
 template class TAO_Notify_ProxySupplier<POA_CosNotifyChannelAdmin::SequenceProxyPushSupplier>;
 template class TAO_Notify_Proxy<POA_CosNotifyChannelAdmin::SequenceProxyPushSupplier>;
 
 #elif defined (ACE_HAS_TEMPLATE_INSTANTIATION_PRAGMA)
-#pragma instantiate TAO_Notify_ProxySupplier<POA_CosNotifyChannelAdmin::SequenceProxyPushSupplier>
 
+#pragma instantiate TAO_Notify_ProxySupplier<POA_CosNotifyChannelAdmin::SequenceProxyPushSupplier>
 #pragma instantiate TAO_Notify_Proxy<POA_CosNotifyChannelAdmin::SequenceProxyPushSupplier>
+
 #endif /*ACE_HAS_EXPLICIT_TEMPLATE_INSTANTIATION */
diff --git a/TAO/orbsvcs/orbsvcs/Notify/Notify_SequenceProxyPushSupplier_i.h b/TAO/orbsvcs/orbsvcs/Notify/Notify_SequenceProxyPushSupplier_i.h
index 5d7a5cc0288..340e4e0ca66 100644
--- a/TAO/orbsvcs/orbsvcs/Notify/Notify_SequenceProxyPushSupplier_i.h
+++ b/TAO/orbsvcs/orbsvcs/Notify/Notify_SequenceProxyPushSupplier_i.h
@@ -58,6 +58,7 @@ class TAO_Notify_Export TAO_Notify_SequenceProxyPushSupplier_i : public TAO_Noti
     CosEventChannelAdmin::TypeError
   ));
 
+  // @@ Pradeep: more indentation problems....
 virtual void disconnect_sequence_push_supplier (
     CORBA::Environment &ACE_TRY_ENV
   )
@@ -65,6 +66,9 @@ virtual void disconnect_sequence_push_supplier (
     CORBA::SystemException
   ));
 
+  // @@ Pradeep: please setup your editor to start 'protected',
+  // 'public' and 'private' on the first column
+
  protected:
   virtual void dispatch_event_i (TAO_Notify_Event &event, CORBA::Environment &ACE_TRY_ENV);
   // Deliver the event to the consumer.
diff --git a/TAO/orbsvcs/orbsvcs/Notify/Notify_StructuredProxyPushConsumer_i.cpp b/TAO/orbsvcs/orbsvcs/Notify/Notify_StructuredProxyPushConsumer_i.cpp
index 79549f861b8..2aa8f3f5e10 100644
--- a/TAO/orbsvcs/orbsvcs/Notify/Notify_StructuredProxyPushConsumer_i.cpp
+++ b/TAO/orbsvcs/orbsvcs/Notify/Notify_StructuredProxyPushConsumer_i.cpp
@@ -5,9 +5,12 @@
 #include "Notify_Event_Manager.h"
 #include "Notify_SupplierAdmin_i.h"
 
+// @@ Pradeep: be systematic about the RCSID
+
 TAO_Notify_StructuredProxyPushConsumer_i::TAO_Notify_StructuredProxyPushConsumer_i (TAO_Notify_SupplierAdmin_i* supplieradmin, TAO_Notify_Resource_Manager* resource_manager)
   : TAO_Notify_ProxyConsumer <POA_CosNotifyChannelAdmin::StructuredProxyPushConsumer> (supplieradmin, resource_manager)
 {
+  // @@ Pradeep: the comments...
   // No-Op.
 }
 
diff --git a/TAO/orbsvcs/orbsvcs/Notify/Notify_StructuredProxyPushSupplier_i.cpp b/TAO/orbsvcs/orbsvcs/Notify/Notify_StructuredProxyPushSupplier_i.cpp
index 6c4174df257..b6381a60485 100644
--- a/TAO/orbsvcs/orbsvcs/Notify/Notify_StructuredProxyPushSupplier_i.cpp
+++ b/TAO/orbsvcs/orbsvcs/Notify/Notify_StructuredProxyPushSupplier_i.cpp
@@ -4,9 +4,14 @@
 #include "Notify_StructuredProxyPushSupplier_i.h"
 #include "Notify_ConsumerAdmin_i.h"
 
-TAO_Notify_StructuredProxyPushSupplier_i::TAO_Notify_StructuredProxyPushSupplier_i (TAO_Notify_ConsumerAdmin_i* consumeradmin, TAO_Notify_Resource_Manager* resource_manager)
+// @@ Pradeep: be systematic about the RCSID
+
+TAO_Notify_StructuredProxyPushSupplier_i::
+     TAO_Notify_StructuredProxyPushSupplier_i (TAO_Notify_ConsumerAdmin_i* consumeradmin,
+                                               TAO_Notify_Resource_Manager* resource_manager)
   :TAO_Notify_ProxySupplier<POA_CosNotifyChannelAdmin::StructuredProxyPushSupplier> (consumeradmin, resource_manager)
 {
+  // @@ Pradeep: more superflous comments..
   //No-Op.
 }
 
diff --git a/TAO/orbsvcs/orbsvcs/Notify/Notify_StructuredPushConsumer.cpp b/TAO/orbsvcs/orbsvcs/Notify/Notify_StructuredPushConsumer.cpp
index cb5e0be5d2c..5c060c351ea 100644
--- a/TAO/orbsvcs/orbsvcs/Notify/Notify_StructuredPushConsumer.cpp
+++ b/TAO/orbsvcs/orbsvcs/Notify/Notify_StructuredPushConsumer.cpp
@@ -1,6 +1,14 @@
 /* -*- C++ -*- $Id$ */
 #include "Notify_StructuredPushConsumer.h"
 
+
+// @@ Pradeep: in general i think that helper classes like this do
+// more harm than help, my experience is that they can hardly cover
+// but a few use cases, and increases confusion among users.
+// It is better to put classes like this in examples and tests, so
+// users can steal the code if they want to.  But then, i'm known to
+// be a minimimalistic bigot ;-) ;-)
+
 TAO_Notify_StructuredPushConsumer::TAO_Notify_StructuredPushConsumer (void)
 {
   // No-Op.
@@ -10,6 +18,9 @@ TAO_Notify_StructuredPushConsumer::~TAO_Notify_StructuredPushConsumer (void)
 {
 }
 
+// @@ Pradeep: do not assume that the user will activate the servant
+// using _this() and changing the default POA, there are many other
+// activation modes.
 void TAO_Notify_StructuredPushConsumer::init (PortableServer::POA_ptr poa, CORBA::Environment & /*ACE_TRY_ENV*/)
 {
   this->default_POA_ = PortableServer::POA::_duplicate (poa);
diff --git a/TAO/orbsvcs/orbsvcs/Notify/Notify_StructuredPushConsumer.h b/TAO/orbsvcs/orbsvcs/Notify/Notify_StructuredPushConsumer.h
index b4c86ed6dea..b86f3ed1ebb 100644
--- a/TAO/orbsvcs/orbsvcs/Notify/Notify_StructuredPushConsumer.h
+++ b/TAO/orbsvcs/orbsvcs/Notify/Notify_StructuredPushConsumer.h
@@ -63,6 +63,9 @@ class TAO_Notify_Export TAO_Notify_StructuredPushConsumer : public POA_CosNotify
   CosNotifyChannelAdmin::StructuredProxyPushSupplier_var proxy_supplier_;
   // The proxy that we are connected to.
 
+  // @@ Pradeep: you may want to be consitent about your field names,
+  // here is <proxy_id> for the supplier is <my_id_>, for the
+  // TAO_Notify_Proxy is just <myID_>....
   CosNotifyChannelAdmin::ProxyID proxy_id_;
   // The proxy_supplier id.
 
diff --git a/TAO/orbsvcs/orbsvcs/Notify/Notify_StructuredPushSupplier.cpp b/TAO/orbsvcs/orbsvcs/Notify/Notify_StructuredPushSupplier.cpp
index a1ac2be87d3..a803241ee5e 100644
--- a/TAO/orbsvcs/orbsvcs/Notify/Notify_StructuredPushSupplier.cpp
+++ b/TAO/orbsvcs/orbsvcs/Notify/Notify_StructuredPushSupplier.cpp
@@ -1,6 +1,9 @@
-/* -*- C++ -*- $Id$ */
+// -*- C++ -*-
+// $Id$
 #include "Notify_StructuredPushSupplier.h"
 
+// @@ Pradeep: the RCSID
+
 TAO_Notify_StructuredPushSupplier::TAO_Notify_StructuredPushSupplier (void)
 {
 }
diff --git a/TAO/orbsvcs/orbsvcs/Notify/Notify_StructuredPushSupplier.h b/TAO/orbsvcs/orbsvcs/Notify/Notify_StructuredPushSupplier.h
index 9bc4e81ebe6..58a9af4ee59 100644
--- a/TAO/orbsvcs/orbsvcs/Notify/Notify_StructuredPushSupplier.h
+++ b/TAO/orbsvcs/orbsvcs/Notify/Notify_StructuredPushSupplier.h
@@ -43,10 +43,14 @@ class TAO_Notify_Export TAO_Notify_StructuredPushSupplier:public POA_CosNotifyCo
   TAO_Notify_StructuredPushSupplier (void);
   // Constructor.
 
-  void init (PortableServer::POA_ptr poa, CORBA::Environment & /*ACE_TRY_ENV*/);
+  void init (PortableServer::POA_ptr poa,
+             CORBA::Environment & /*ACE_TRY_ENV*/);
   // Init
 
-  void connect (CosNotifyChannelAdmin::SupplierAdmin_ptr supplier_admin, CORBA::Environment &ACE_TRY_ENV);
+  // @@ Pradeep: try to cleanup the code you cut & paste, it is was
+  // really hard to read.
+  void connect (CosNotifyChannelAdmin::SupplierAdmin_ptr supplier_admin,
+                CORBA::Environment &ACE_TRY_ENV);
   // Connect the Supplier to the EventChannel.
   // Creates a new proxy supplier and connects to it.
 
@@ -64,6 +68,7 @@ class TAO_Notify_Export TAO_Notify_StructuredPushSupplier:public POA_CosNotifyCo
   // = ServantBase operations
   PortableServer::POA_ptr _default_POA (CORBA::Environment &env);
 
+  // @@ Any reasons why this is public?
   CosNotifyChannelAdmin::ProxyID my_id_;
   // This supplier's id.
 
diff --git a/TAO/orbsvcs/orbsvcs/Notify/Notify_SupplierAdmin_i.cpp b/TAO/orbsvcs/orbsvcs/Notify/Notify_SupplierAdmin_i.cpp
index 7be518f899b..d7564e6f5a8 100644
--- a/TAO/orbsvcs/orbsvcs/Notify/Notify_SupplierAdmin_i.cpp
+++ b/TAO/orbsvcs/orbsvcs/Notify/Notify_SupplierAdmin_i.cpp
@@ -39,6 +39,8 @@ TAO_Notify_SupplierAdmin_i::init (CosNotifyChannelAdmin::AdminID myID,
                                   PortableServer::POA_ptr my_POA,
                                   CORBA::Environment &ACE_TRY_ENV)
 {
+  // @@ Pradeep: please use the this-> style to access fields...
+
   my_POA_ = PortableServer::POA::_duplicate (my_POA);
 
   this->proxy_pushconsumer_POA_ = this->resource_manager_->
@@ -54,11 +56,18 @@ TAO_Notify_SupplierAdmin_i::init (CosNotifyChannelAdmin::AdminID myID,
 CosNotifyChannelAdmin::SupplierAdmin_ptr
 TAO_Notify_SupplierAdmin_i::get_ref (CORBA::Environment &ACE_TRY_ENV)
 {
+  // @@ Pradeep: if you get a chance to quantify this stuff you will
+  // notice that this is a very expensive operation, you may want to
+  // cache the result if it is invoked very often...
+
   return CosNotifyChannelAdmin::SupplierAdmin
     ::_narrow (this->resource_manager_->
                servant_to_reference (this->my_POA_.in (), this, ACE_TRY_ENV));
 }
 
+// @@ Pradeep: it is possible that you want methods like this
+// inlined...
+
 TAO_Notify_Event_Manager*
 TAO_Notify_SupplierAdmin_i::get_event_manager (void)
 {
@@ -113,6 +122,8 @@ TAO_Notify_SupplierAdmin_i::MyOperator (CORBA::Environment &/*ACE_TRY_ENV*/)
                    CORBA::SystemException
                    ))
 {
+  // Pradeep: we don't use intercaps for field names, but '_' to
+  // separate words, and they are all lowercase....
   return myOperator_;
 }
 
@@ -122,6 +133,10 @@ TAO_Notify_SupplierAdmin_i::pull_consumers (CORBA::Environment& ACE_TRY_ENV)
                    CORBA::SystemException
                    ))
 {
+  // @@ Pradeep: there is no rush to implement pull, but look at the
+  // code in the new CosEC, we may need to start thinking about pull,
+  // and how can we reduce duplicated code for both pull and push
+  // models.
   ACE_THROW_RETURN (CORBA::NO_IMPLEMENT (), 0);
 }
 
@@ -401,6 +416,8 @@ TAO_Notify_SupplierAdmin_i::obtain_pull_consumer (CORBA::Environment &ACE_TRY_EN
                    CORBA::SystemException
                    ))
 {
+  // @@ Pradeep: you may want to group all the Pull methods together,
+  // it would be easier to identify them or read the code that way.
   ACE_THROW_RETURN (CORBA::NO_IMPLEMENT (),
                     CosEventChannelAdmin::ProxyPullConsumer::_nil ());
 }
diff --git a/TAO/orbsvcs/orbsvcs/Notify/Notify_SupplierAdmin_i.h b/TAO/orbsvcs/orbsvcs/Notify/Notify_SupplierAdmin_i.h
index 72a00753339..a58e11d1b44 100644
--- a/TAO/orbsvcs/orbsvcs/Notify/Notify_SupplierAdmin_i.h
+++ b/TAO/orbsvcs/orbsvcs/Notify/Notify_SupplierAdmin_i.h
@@ -20,6 +20,9 @@
 #define TAO_NOTIFY_SUPPLIERADMIN_I_H
 #include "ace/pre.h"
 
+// @@ Pradeep: i believe that the #pragma once guideline is still in
+// effect.
+
 #include "Notify_ID_Pool_T.h"
 #include "Notify_QoSAdmin_i.h"
 #include "Notify_FilterAdmin_i.h"
@@ -46,7 +49,11 @@ class TAO_Notify_Export TAO_Notify_SupplierAdmin_i : public POA_CosNotifyChannel
   //
 
 public:
-  TAO_Notify_SupplierAdmin_i (TAO_Notify_EventChannel_i* myChannel, TAO_Notify_Resource_Manager* resource_manager);
+  // @@ Pradeep: you may want to drop the _i suffix, it buys you
+  // nothing..
+
+  TAO_Notify_SupplierAdmin_i (TAO_Notify_EventChannel_i* myChannel,
+                              TAO_Notify_Resource_Manager* resource_manager);
   // Constructor
   // <myChannel> is this objects parent.
 
@@ -59,10 +66,13 @@ public:
              CORBA::Environment &ACE_TRY_ENV);
   //Initialize the Supplier Admin.
 
+  // @@ Pradeep: could this method be const?  Try to use const
+  // operations when possible.
   TAO_Notify_FilterAdmin_i& get_filter_admin (void);
   // Get our filter admin.
 
-  void deactivate_proxy_pushconsumer (PortableServer::Servant servant, CORBA::Environment &ACE_TRY_ENV);
+  void deactivate_proxy_pushconsumer (PortableServer::Servant servant,
+                                      CORBA::Environment &ACE_TRY_ENV);
   // Deactivate servant from <proxy_pushconsumer_POA_>.
 
   void proxy_pushconsumer_destroyed (CosNotifyChannelAdmin::ProxyID proxyID);
@@ -82,12 +92,14 @@ public:
   TAO_Notify_Event_Manager* get_event_manager (void);
   // Accesor for the event manager.
 
-virtual CosNotifyChannelAdmin::EventChannel_ptr MyChannel (
-    CORBA::Environment &ACE_TRY_ENV
-  )
-  ACE_THROW_SPEC ((
-    CORBA::SystemException
-  ));
+  // @@ Pradeep: Don't forget to indent this stuff, at the very least
+  // it should not start in the first column!
+  virtual CosNotifyChannelAdmin::EventChannel_ptr MyChannel (
+      CORBA::Environment &ACE_TRY_ENV
+    )
+    ACE_THROW_SPEC ((
+      CORBA::SystemException
+    ));
 
 virtual CosNotifyChannelAdmin::InterFilterGroupOperator MyOperator (
     CORBA::Environment &ACE_TRY_ENV
@@ -238,6 +250,10 @@ virtual CosEventChannelAdmin::ProxyPullConsumer_ptr obtain_pull_consumer (
 
 protected:
   // = Helper methods
+  // @@ Pradeep: you don't need to provide default values for
+  // private, protected or implementation methods.  In fact it is a
+  // good idea *not* to provide the default value.  That way you make
+  // sure that you are passing it around.
  void cleanup_i (CORBA::Environment &ACE_TRY_ENV = TAO_default_environment ());
   // Cleanup all resources used by this object.
 
@@ -270,12 +286,17 @@ protected:
   // The POA in which all our push consumers live.
   // We create and own this POA.
 
+  // @@ Pradeep: you may want to use a typedef for that template.
   TAO_Notify_ID_Pool_Ex<CosNotifyChannelAdmin::ProxyID,
     CosNotifyChannelAdmin::ProxyIDSeq> proxy_pushconsumer_ids_;
   // Id generator for proxy push consumers.
 
   CORBA::Boolean is_destroyed_;
   // Are we dead?
+  // @@ Pradeep: you may want to explain why such a flag is needed. I
+  // assume it is set when the a request to destroy the object
+  // arrives, but you must perform some cleanup, meanwhile any new
+  // requests are rejected, right?
 
   TAO_Notify_QoSAdmin_i qos_admin_;
   // Handle QoS admin methods.
diff --git a/TAO/orbsvcs/orbsvcs/Notify/Notify_Types.cpp b/TAO/orbsvcs/orbsvcs/Notify/Notify_Types.cpp
index 1dacad8e158..ef35f19a838 100644
--- a/TAO/orbsvcs/orbsvcs/Notify/Notify_Types.cpp
+++ b/TAO/orbsvcs/orbsvcs/Notify/Notify_Types.cpp
@@ -15,7 +15,8 @@ TAO_Notify_EventType::TAO_Notify_EventType (void)
   // No-Op.
 }
 
-TAO_Notify_EventType::TAO_Notify_EventType (const char* domain_name, const char* type_name)
+TAO_Notify_EventType::TAO_Notify_EventType (const char* domain_name,
+                                            const char* type_name)
 {
   this->event_type_.type_name = type_name;
   this->event_type_.domain_name = domain_name;
@@ -25,6 +26,9 @@ TAO_Notify_EventType::TAO_Notify_EventType (const char* domain_name, const char*
 TAO_Notify_EventType::TAO_Notify_EventType (const CosNotification::EventType& event_type)
 {
   // @@ Check it these dups are indeed required.
+  // @@ Pradeep: do not live with broken windows, if the code did not
+  // need the dups then just remove them!
+
   this->event_type_.type_name = event_type.type_name.in ();
   //CORBA::string_dup (event_type.type_name.in ());
   this->event_type_.domain_name = event_type.domain_name.in ();
@@ -34,18 +38,32 @@ TAO_Notify_EventType::TAO_Notify_EventType (const CosNotification::EventType& ev
 
 TAO_Notify_EventType::~TAO_Notify_EventType ()
 {
+  // @@ Pradeep:
   // No-Op.
 }
 
 u_long
 TAO_Notify_EventType::hash (void) const
 {
+  // @@ Pradeep: this is an excellent candidate for an inline
+  // function. Get in the habit of creating .h, .cpp and .i files,
+  // even if initially the .i file is empty, that way it is easier to
+  // do this stuff.
   return this->hash_value_;
 }
 
 void
 TAO_Notify_EventType::recompute_hash (void)
 {
+  // @@ Pradeep: this code is bound to crash someday if the strings
+  // are too long....  See if the hash_pjw() function can be modified
+  // to take accumulate multiple strings, as in:
+  //   hash = ACE::hash_pjw_accummulate (0, str1);
+  //   hash = ACE::hash_pjw_accummulate (hash, str2);
+  //
+  // @@ Or use grow the buffer when needed, or just add the two hash
+  // values or something, but fix this code!
+  // 
   char buffer[BUFSIZ];
   ACE_OS::strcpy (buffer, this->event_type_.domain_name.in ());
   ACE_OS::strcat (buffer, this->event_type_.type_name.in ());
@@ -64,6 +82,12 @@ TAO_Notify_EventType::operator=(const CosNotification::EventType& rhs)
 int
 TAO_Notify_EventType::operator==(const TAO_Notify_EventType& rhs) const
 {
+  // @@ Pradeep: what kind of hack is this?  What you should do is:
+  // if (this->hash () != rhs.hash ())
+  //    return 0;
+  // else
+  //   compare the strings!
+  //
   if (this->hash () == rhs.hash ())
     return 1;
   else
@@ -91,10 +115,18 @@ TAO_Notify_EventType::get_native (void) const
   return event_type_;
 }
 
+// ****************************************************************
+
 // = Any Event Type.
 
 TAO_Notify_Any::TAO_Notify_Any (void)
 {
+  // @@ Pradeep: get in the habit of initializing in the
+  // initialization section.  This is a field of the base class (beats
+  // me why it is there), but then you should provide a protected
+  // constructor to initialize it!
+
+  // @@ Pradeep: remember it is 'this->is_owner_'
   is_owner_ = 0;
 }
 
@@ -109,6 +141,8 @@ TAO_Notify_Any::TAO_Notify_Any (const CORBA::Any & data)
 
 TAO_Notify_Any::~TAO_Notify_Any ()
 {
+  // @@ Pradeep: please be consistent, use this-> for *all* fields.
+
   if (this->is_owner_)
     delete data_;
 }
@@ -121,6 +155,8 @@ TAO_Notify_Any::clone (void)
 
   if (this->is_owner_)
     {
+      // @@ Are you sure this is the right way to clone?  You are
+      // stealing the data from the original class...
       this->is_owner_ = 0;
       clone->data_ = this->data_;
       clone->is_owner_ = 1;
@@ -162,19 +198,22 @@ TAO_Notify_Any::event_type (void) const
 }
 
 CORBA::Boolean
-TAO_Notify_Any::do_match (CosNotifyFilter::Filter_ptr filter, CORBA::Environment &ACE_TRY_ENV) const
+TAO_Notify_Any::do_match (CosNotifyFilter::Filter_ptr filter,
+                          CORBA::Environment &ACE_TRY_ENV) const
 {
   return filter->match (*this->data_, ACE_TRY_ENV);
 }
 
 void
-TAO_Notify_Any::do_push (CosEventComm::PushConsumer_ptr consumer, CORBA::Environment &ACE_TRY_ENV) const
+TAO_Notify_Any::do_push (CosEventComm::PushConsumer_ptr consumer,
+                         CORBA::Environment &ACE_TRY_ENV) const
 {
   consumer->push (*this->data_, ACE_TRY_ENV);
 }
 
 void
-TAO_Notify_Any::do_push (CosNotifyComm::StructuredPushConsumer_ptr consumer, CORBA::Environment &ACE_TRY_ENV) const
+TAO_Notify_Any::do_push (CosNotifyComm::StructuredPushConsumer_ptr consumer,
+                         CORBA::Environment &ACE_TRY_ENV) const
 {
   // translation pg. 28
   CosNotification::StructuredEvent event;
@@ -185,8 +224,13 @@ TAO_Notify_Any::do_push (CosNotifyComm::StructuredPushConsumer_ptr consumer, COR
   consumer->push_structured_event (event, ACE_TRY_ENV);
 }
 
+// ****************************************************************
+
 // = TAO_Notify_StructuredEvent
 
+// @@ Pradeep: many of the same comments that i made for
+// TAO_Notify_Any apply here too.
+
 TAO_Notify_StructuredEvent::TAO_Notify_StructuredEvent (void)
 {
   is_owner_ = 0;
@@ -255,13 +299,15 @@ TAO_Notify_StructuredEvent::event_type (void) const
 }
 
 CORBA::Boolean
-TAO_Notify_StructuredEvent::do_match (CosNotifyFilter::Filter_ptr filter, CORBA::Environment &ACE_TRY_ENV) const
+TAO_Notify_StructuredEvent::do_match (CosNotifyFilter::Filter_ptr filter,
+                                      CORBA::Environment &ACE_TRY_ENV) const
 {
   return filter->match_structured (*this->data_, ACE_TRY_ENV);
 }
 
 void
-TAO_Notify_StructuredEvent::do_push (CosEventComm::PushConsumer_ptr consumer, CORBA::Environment &ACE_TRY_ENV) const
+TAO_Notify_StructuredEvent::do_push (CosEventComm::PushConsumer_ptr consumer,
+                                     CORBA::Environment &ACE_TRY_ENV) const
 {
   // translation pg. 28
   CORBA::Any any;
@@ -277,6 +323,8 @@ TAO_Notify_StructuredEvent::do_push (CosNotifyComm::StructuredPushConsumer_ptr c
   consumer->push_structured_event (*this->data_, ACE_TRY_ENV);
 }
 
+// ****************************************************************
+
 // = EVENTTYPE_LIST
 void
 EVENTTYPE_LIST::populate (CosNotification::EventTypeSeq& event_type_seq)
diff --git a/TAO/orbsvcs/orbsvcs/Notify/Notify_Types.h b/TAO/orbsvcs/orbsvcs/Notify/Notify_Types.h
index df62e4aeb51..6d573665cb6 100644
--- a/TAO/orbsvcs/orbsvcs/Notify/Notify_Types.h
+++ b/TAO/orbsvcs/orbsvcs/Notify/Notify_Types.h
@@ -26,6 +26,7 @@
 class TAO_Notify_Event_Listener;
 class TAO_Notify_Update_Listener;
 
+// @@ Pradeep: do not forget the TAO_***_Export macros...
 class TAO_Notify_EventType
 {
   // = TITLE
@@ -78,12 +79,22 @@ protected:
   // A special event type
 };
 
+// ****************************************************************
+// @@ Pradeep: please remember to separate your classes with a line
+// like the one above.  Or better yet, do not put multiple classes in
+// the same file.
+
+// @@ Pradeep: more export macro madness
 class TAO_Notify_Event
 {
   // = TITLE
   //   TAO_Notify_Event
   //
   // = DESCRIPTION
+  //   @@ Pradeep: Could you explain in more detail what do this
+  //   abstraction is there for?  Is it to treat different event
+  //   types, like anys, structured events, etc. homogenously?
+  //
   //   Abstraction for an event
   //
 public:
@@ -106,13 +117,17 @@ public:
   virtual void do_push (CosEventComm::PushConsumer_ptr consumer, CORBA::Environment &ACE_TRY_ENV) const = 0;
   virtual void do_push (CosNotifyComm::StructuredPushConsumer_ptr consumer, CORBA::Environment &ACE_TRY_ENV) const = 0;
   // Push self to <consumer>
+
 protected:
   CORBA::Boolean is_owner_;
   // Do we own the data.
 };
 
+// ****************************************************************
+
 class TAO_Notify_Any : public TAO_Notify_Event
 {
+  // @@ Pradeep: please document this class...
 public:
   TAO_Notify_Any (void);
   TAO_Notify_Any (const CORBA::Any & data);
@@ -133,8 +148,11 @@ protected:
   // The data
 };
 
+// ****************************************************************
+
 class TAO_Notify_StructuredEvent : public TAO_Notify_Event
 {
+  // @@ Pradeep: please document this class.
 public:
   TAO_Notify_StructuredEvent (void);
   TAO_Notify_StructuredEvent (const CosNotification::StructuredEvent & notification);
@@ -157,6 +175,9 @@ protected:
   // The event type of <data_>
 };
 
+// ****************************************************************
+
+// @@ Pradeep: please change the name of this class...
 class EVENTTYPE_LIST : public ACE_Unbounded_Set <TAO_Notify_EventType>
 {
   // = TITLE
@@ -179,6 +200,13 @@ public:
   // remove the contents of <event_type_seq> from this object.
 };
 
+// ****************************************************************
+
+// @@ Pradeep, please don't use all CAPS for types, we use all caps
+// for macros only.
+// @@ Pradeep: typedefs or not they should be prefixed, otherwise you
+// are polluting the namespace.
+
 // = typedefs
 typedef ACE_Unbounded_Set<TAO_Notify_Event_Listener*> EVENT_LISTENER_LIST;
 // A list of event listeners that are looking for the same event type.