twindle-co / twindle

Twindle - an open source project for beginners. Converting twitter threads to pdf, epub, and mobi format to be read by Kindle.
https://www.twindle.co
MIT License
134 stars 133 forks source link

Twitter refactor part: Is this granularity necessary #710

Closed PuruVJ closed 3 years ago

PuruVJ commented 3 years ago

I saw a file structure like this in the Twitter refactor:

image

And the contents look like this. image

My question: Is it really necessary to split them up so much? For example, It'll be better to import format directly from date-fns, and it'll convey meaning that it is related to Date formatting only, cuz there might be more formatters later on.

And const {format} = require('../utils/date') would be a bit too confusing at first.

And if we're doing this, we better do it right. Let's change the file a bit:

const { format } = require("date-fns");

const formatTimestamp = (timestamp) => format(new Date(timestamp), 'MMM d, yyyy')

module.exports = {
  formatTimestamp,
};

What are your thoughts @johnjacobkenny @Mira-Alf ?

johnjacobkenny commented 3 years ago

Yep fully agree. Go for it :+1:

Mira-Alf commented 3 years ago

I didn't see this level of modularity. It makes sense to clean it up.

PuruVJ commented 3 years ago

@Mira-Alf No issue. I've replaced existing one with the much better snippet I provided above

johnjacobkenny commented 3 years ago

@Mira-Alf @PuruVJ The idea overall was to have a facade around the external library, so that in case we need to switch it out later, we can easily by making changes in a single file.

PuruVJ commented 3 years ago

Yes! I agree. I've added cleanup logic, and some more stuff in that single file. kenny's idea was really good.