Better name for PQsslAttributes()

Started by Lars Kanisabout 10 years ago10 messages
#1Lars Kanis
lars@greiz-reinsdorf.de

As a co-maintainer of the PostgreSQL adapter for Ruby, I would like to
bridge the new SSL related functions to Ruby methods. However I wonder
whether PQsslAttributes() is the best name for the function. Based on
this name, I would expect to get key+value pairs instead of only the
keys. IMHO PQsslAttributeNames() would express better, what the function
does.

--
Kind Regards,
Lars

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#2Heikki Linnakangas
hlinnaka@iki.fi
In reply to: Lars Kanis (#1)
Re: Better name for PQsslAttributes()

On 11/06/2015 11:31 PM, Lars Kanis wrote:

As a co-maintainer of the PostgreSQL adapter for Ruby, I would like to
bridge the new SSL related functions to Ruby methods. However I wonder
whether PQsslAttributes() is the best name for the function. Based on
this name, I would expect to get key+value pairs instead of only the
keys. IMHO PQsslAttributeNames() would express better, what the function
does.

Hmm, I think you're right.

The question is, do we want to still change it? It's a new function in
9.5, and we're just about to enter beta, so I guess we could, although
there might already be applications out there using it. If we do want to
rename it, now is the last chance to do it.

Thoughts? I'm leaning towards changing it now.

- Heikki

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#3Magnus Hagander
magnus@hagander.net
In reply to: Heikki Linnakangas (#2)
Re: Better name for PQsslAttributes()

On Fri, Nov 6, 2015 at 10:38 PM, Heikki Linnakangas <hlinnaka@iki.fi> wrote:

On 11/06/2015 11:31 PM, Lars Kanis wrote:

As a co-maintainer of the PostgreSQL adapter for Ruby, I would like to
bridge the new SSL related functions to Ruby methods. However I wonder
whether PQsslAttributes() is the best name for the function. Based on
this name, I would expect to get key+value pairs instead of only the
keys. IMHO PQsslAttributeNames() would express better, what the function
does.

Hmm, I think you're right.

The question is, do we want to still change it? It's a new function in
9.5, and we're just about to enter beta, so I guess we could, although
there might already be applications out there using it. If we do want to
rename it, now is the last chance to do it.

Thoughts? I'm leaning towards changing it now.

Uh, just to be clear, we been in beta for a month now, beta1 was released
Oct 8. We are not just about to enter it...

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

#4Peter Geoghegan
pg@heroku.com
In reply to: Heikki Linnakangas (#2)
Re: Better name for PQsslAttributes()

On Fri, Nov 6, 2015 at 1:38 PM, Heikki Linnakangas <hlinnaka@iki.fi> wrote:

Thoughts? I'm leaning towards changing it now.

+1 to the idea of changing it.

--
Peter Geoghegan

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#5Tom Lane
tgl@sss.pgh.pa.us
In reply to: Heikki Linnakangas (#2)
Re: Better name for PQsslAttributes()

Heikki Linnakangas <hlinnaka@iki.fi> writes:

On 11/06/2015 11:31 PM, Lars Kanis wrote:

As a co-maintainer of the PostgreSQL adapter for Ruby, I would like to
bridge the new SSL related functions to Ruby methods. However I wonder
whether PQsslAttributes() is the best name for the function. Based on
this name, I would expect to get key+value pairs instead of only the
keys. IMHO PQsslAttributeNames() would express better, what the function
does.

Hmm, I think you're right.

The question is, do we want to still change it? It's a new function in
9.5, and we're just about to enter beta, so I guess we could, although
there might already be applications out there using it. If we do want to
rename it, now is the last chance to do it.

Thoughts? I'm leaning towards changing it now.

I agree that this is about the last possible chance to rename it, if
indeed that chance is not already past.

However, it seems somewhat unlikely that anyone would be depending on the
thing already, so I think probably we could get away with renaming it.

+0.5 or so to changing it. But if we do, it has to happen before Monday.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#6Lars Kanis
lars@greiz-reinsdorf.de
In reply to: Tom Lane (#5)
2 attachment(s)
Re: Better name for PQsslAttributes()

Thank you for the quick response! Attached the last minute - now or
never patch to change the function name.

In addition I perceived a small inconsistency with the naming of the
SGML id of PQsslAttribute. This is addressed in the second patch file.

--
Kind Regards,
Lars

Attachments:

0001-Lowercase-pqsslAttribute-in-SGML-id-for-consistency.patchtext/x-patch; name=0001-Lowercase-pqsslAttribute-in-SGML-id-for-consistency.patchDownload
From 5b7f116de237df5404938fcb3a722c071d0ee120 Mon Sep 17 00:00:00 2001
From: Lars Kanis <lars@greiz-reinsdorf.de>
Date: Sat, 7 Nov 2015 21:00:21 +0100
Subject: [PATCH] Lowercase pqsslAttribute in SGML-id for consistency.

---
 doc/src/sgml/libpq.sgml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index 0ee018e..fdb06c3 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -1873,7 +1873,7 @@ int PQsslInUse(const PGconn *conn);
      </listitem>
     </varlistentry>
 
-    <varlistentry id="libpq-pqsslAttribute">
+    <varlistentry id="libpq-pqsslattribute">
      <term><function>PQsslAttribute</function><indexterm><primary>PQsslAttribute</></></term>
      <listitem>
       <para>
-- 
2.1.4

0001-Rename-PQsslAttributes-to-PQsslAttributeNames.patchtext/x-patch; name=0001-Rename-PQsslAttributes-to-PQsslAttributeNames.patchDownload
From 2d86b4d785bb5b53c779b79a8010e7d7f6cbb9b5 Mon Sep 17 00:00:00 2001
From: Lars Kanis <lars@greiz-reinsdorf.de>
Date: Sat, 7 Nov 2015 20:42:43 +0100
Subject: [PATCH] Rename PQsslAttributes() to PQsslAttributeNames().

The name PQsslAttributes suggested, the function would return
attribute values or key/value pairs, but it returns keys only.
PQsslAttributeNames makes more clear, what the function is about.
---
 doc/src/sgml/libpq.sgml                  | 6 +++---
 src/interfaces/libpq/exports.txt         | 2 +-
 src/interfaces/libpq/fe-secure-openssl.c | 2 +-
 src/interfaces/libpq/fe-secure.c         | 2 +-
 src/interfaces/libpq/libpq-fe.h          | 2 +-
 5 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index 0ee018e..7c31f42 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -1947,13 +1947,13 @@ const char *PQsslAttribute(const PGconn *conn, const char *attribute_name);
      </listitem>
     </varlistentry>
 
-    <varlistentry id="libpq-pqsslattributes">
-     <term><function>PQsslAttributes</function><indexterm><primary>PQsslAttributes</></></term>
+    <varlistentry id="libpq-pqsslattributenames">
+     <term><function>PQsslAttributeNames</function><indexterm><primary>PQsslAttributeNames</></></term>
      <listitem>
       <para>
        Return an array of SSL attribute names available. The array is terminated by a NULL pointer.
 <synopsis>
-const char **PQsslAttributes(const PGconn *conn);
+const char **PQsslAttributeNames(const PGconn *conn);
 </synopsis>
       </para>
      </listitem>
diff --git a/src/interfaces/libpq/exports.txt b/src/interfaces/libpq/exports.txt
index 0bbcae5..c69a4d5 100644
--- a/src/interfaces/libpq/exports.txt
+++ b/src/interfaces/libpq/exports.txt
@@ -167,6 +167,6 @@ lo_truncate64             164
 PQconninfo                165
 PQsslInUse                166
 PQsslStruct               167
-PQsslAttributes           168
+PQsslAttributeNames       168
 PQsslAttribute            169
 PQsetErrorContextVisibility 170
diff --git a/src/interfaces/libpq/fe-secure-openssl.c b/src/interfaces/libpq/fe-secure-openssl.c
index 4b2a324..e88a7ee 100644
--- a/src/interfaces/libpq/fe-secure-openssl.c
+++ b/src/interfaces/libpq/fe-secure-openssl.c
@@ -1533,7 +1533,7 @@ PQsslStruct(PGconn *conn, const char *struct_name)
 }
 
 const char **
-PQsslAttributes(PGconn *conn)
+PQsslAttributeNames(PGconn *conn)
 {
 	static const char *result[] = {
 		"library",
diff --git a/src/interfaces/libpq/fe-secure.c b/src/interfaces/libpq/fe-secure.c
index db91e52..ed1342e 100644
--- a/src/interfaces/libpq/fe-secure.c
+++ b/src/interfaces/libpq/fe-secure.c
@@ -409,7 +409,7 @@ PQsslAttribute(PGconn *conn, const char *attribute_name)
 }
 
 const char **
-PQsslAttributes(PGconn *conn)
+PQsslAttributeNames(PGconn *conn)
 {
 	static const char *result[] = {NULL};
 
diff --git a/src/interfaces/libpq/libpq-fe.h b/src/interfaces/libpq/libpq-fe.h
index 828c533..eddb817 100644
--- a/src/interfaces/libpq/libpq-fe.h
+++ b/src/interfaces/libpq/libpq-fe.h
@@ -329,7 +329,7 @@ extern int	PQsetClientEncoding(PGconn *conn, const char *encoding);
 extern int	PQsslInUse(PGconn *conn);
 extern void *PQsslStruct(PGconn *conn, const char *struct_name);
 extern const char *PQsslAttribute(PGconn *conn, const char *attribute_name);
-extern const char **PQsslAttributes(PGconn *conn);
+extern const char **PQsslAttributeNames(PGconn *conn);
 
 /* Get the OpenSSL structure associated with a connection. Returns NULL for
  * unencrypted connections or if any other TLS library is in use. */
-- 
2.1.4

#7Tom Lane
tgl@sss.pgh.pa.us
In reply to: Tom Lane (#5)
Re: Better name for PQsslAttributes()

Tom Lane <tgl@sss.pgh.pa.us> writes:

Heikki Linnakangas <hlinnaka@iki.fi> writes:

The question is, do we want to still change it? It's a new function in
9.5, and we're just about to enter beta, so I guess we could, although
there might already be applications out there using it. If we do want to
rename it, now is the last chance to do it.

+0.5 or so to changing it. But if we do, it has to happen before Monday.

Are we doing this (seems like nobody objected), and if so who's going to
make it happen? Time grows short.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#8Tom Lane
tgl@sss.pgh.pa.us
In reply to: Lars Kanis (#6)
Re: Better name for PQsslAttributes()

Lars Kanis <lars@greiz-reinsdorf.de> writes:

Thank you for the quick response! Attached the last minute - now or
never patch to change the function name.

Ah, thanks for doing the legwork. It's pretty late in the day Heikki's
time, so I'll review and hopefully push this.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#9Tom Lane
tgl@sss.pgh.pa.us
In reply to: Lars Kanis (#6)
Re: Better name for PQsslAttributes()

Lars Kanis <lars@greiz-reinsdorf.de> writes:

Thank you for the quick response! Attached the last minute - now or
never patch to change the function name.

Pushed. I took the opportunity to fix the const-ness annotation of the
function result type, too.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#10Lars Kanis
lars@greiz-reinsdorf.de
In reply to: Tom Lane (#9)
Re: Better name for PQsslAttributes()

Am 07.11.2015 um 22:14 schrieb Tom Lane:

Pushed. I took the opportunity to fix the const-ness annotation of the
function result type, too.

Great, thank you!

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers