Opened 3 years ago

Closed 2 years ago

#24714 closed defect (implemented)

rename conn->timestamp_lastwritten to conn->timestamp_lastwritable

Reported by: arma Owned by: valentecaio
Priority: Medium Milestone: Tor: 0.3.4.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: easy, refactor
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:


"lastwritten" sure sounds like we actually flushed something or we actually added a new thing to write.

But actually,

  time_t timestamp_lastwritten; /**< When was the last time libevent said we
                                 * could write? */

It seems that we changed our definition of this word somewhere along the line, but we left the old word in place. How confusing.

We might want to change lastread to lastreadable too.

Child Tickets

Change History (10)

comment:1 Changed 3 years ago by dgoulet

Keywords: easy refactor added
Milestone: Tor: 0.3.3.x-finalTor: 0.3.4.x-final

comment:2 Changed 3 years ago by valentecaio

I totally agree it sounds like we flushed something... But maybe timestamp_lastwritable is neither clear enough.
What do you think about timestamp_last_write_permission ?

Last edited 3 years ago by valentecaio (previous) (diff)

comment:3 Changed 3 years ago by teor

"write permission" means something very specific in a systems context.
If we really need a specific term, try "last write allowed" and "last read allowed".

comment:4 Changed 3 years ago by valentecaio

thanks for your answer, teor, I'll work on it.

comment:5 Changed 3 years ago by arma

Right -- "writable" is a technical term because it's what select and poll say when you ask about a socket.

But I guess there are other options we could use too, like "last_ready_for_write" or "last_write_allowed", if we want to use more characters. :)

comment:6 Changed 3 years ago by valentecaio

Owner: set to valentecaio
Status: newassigned

comment:7 Changed 3 years ago by valentecaio

I've changed both fields as following:

timestamp_lastwritten -> timestamp_last_write_allowed
timestamp_lastread -> timestamp_last_read_allowed

I'm a beginner so excuse me for any possible mistake, and please review it carefully.
I would be happy to hear any possible suggestion.

The code is available on:

Last edited 3 years ago by valentecaio (previous) (diff)

comment:8 Changed 3 years ago by valentecaio

Status: assignedneeds_review

comment:9 Changed 3 years ago by nickm

Status: needs_reviewmerge_ready

This looks good to me! I'll merge it once the 0.3.4.x merge window opens.

comment:10 Changed 2 years ago by nickm

Resolution: implemented
Status: merge_readyclosed

Merged! Thanks again!

Note: See TracTickets for help on using tickets.