Opened 9 months ago

Closed 7 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:

Description

"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 8 months ago by dgoulet

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

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

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

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

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

Owner: set to valentecaio
Status: newassigned

comment:7 Changed 8 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:
https://github.com/valentecaio/torproject-tor/commit/7884ce76e100abc220ddedb76c9b98d22d66d645

Last edited 8 months ago by valentecaio (previous) (diff)

comment:8 Changed 8 months ago by valentecaio

Status: assignedneeds_review

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

Resolution: implemented
Status: merge_readyclosed

Merged! Thanks again!

Note: See TracTickets for help on using tickets.