Opened 2 years ago

Closed 21 months 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 22 months ago by dgoulet

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

comment:2 Changed 22 months 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 22 months ago by valentecaio (previous) (diff)

comment:3 Changed 22 months 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 22 months ago by valentecaio

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

comment:5 Changed 22 months 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 22 months ago by valentecaio

Owner: set to valentecaio
Status: newassigned

comment:7 Changed 22 months 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:

Version 0, edited 22 months ago by valentecaio (next)

comment:8 Changed 22 months ago by valentecaio

Status: assignedneeds_review

comment:9 Changed 22 months 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 21 months ago by nickm

Resolution: implemented
Status: merge_readyclosed

Merged! Thanks again!

Note: See TracTickets for help on using tickets.