Skip to content

Update scheduled.js #9

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
unbeschwert wants to merge 2 commits into
base: develop
Choose a base branch
from
Open

Update scheduled.js #9

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f6a4bb9
Update scheduled.js
unbeschwert Jun 1, 2021
6ef9b02
Update scheduled.js
unbeschwert Jun 3, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions src/scheduled.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,13 @@ const readLobstersRss = (event) => {
lastSeen = publishedAt;

event.logger.info('Broadcasting story.', {itemDate: item.isoDate, itemGuid: item.guid, lastSeen});
event.reply(`${item.title} [${item.categories.join(' ')}] (${username}) ${item.guid}`);

if(item.link === item.comments) {
event.reply(`${item.title} [${item.categories.join(' ')}] (${username}) ${item.guid}`);
}
else {
event.reply(`${item.title} {${item.link}} [${item.categories.join(' ')}] (${username}) ${item.guid}`);
Copy link
Copy Markdown
Member

@hmadison hmadison Jun 2, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we check the length of the message before we send it? I feel like adding the URL can create a message that's too long for IRC.

Copy link
Copy Markdown
Author

@unbeschwert unbeschwert Jun 3, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @hmadison ,

Is the check really necessary ? Recently I got a list of all URLs posted to lobsters from @pushcx and I created a histogram of these URL lengths which you can find below. Most of the URLs lengths are in range of 30 to 80 characters which seems to be fine IMHO.

Figure_1

Copy link
Copy Markdown
Member

@hmadison hmadison Jun 3, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pulling the maximum lengths from the schema.rb, we have an upper bound of 707 characters. IRC's max length is 510.

Copy link
Copy Markdown
Author

@unbeschwert unbeschwert Jun 3, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah okay yes. In that case we need to add checks for length. I'll make the necessary changes. Please do make sure to run some tests on them since I am not familiar with javascript

Edit: @hmadison I just added a new commit that checks if length of reply is greater than 510. I hope this is correct. If it needs some change, please let me know :)

One other idea I had in mind was to use some sort of URL shortnening service. Although they all required paid plans and seemed a bit too much. If its interests you, I can also look it up :)

Copy link
Copy Markdown

@winks winks Dec 3, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

git.io should work if desired - https://github.blog/2011-11-10-git-io-github-url-shortener/

Copy link
Copy Markdown
Member

@hmadison hmadison Dec 3, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@winks git.io is for GitHub controlled properties.

}
}
});

Expand All @@ -41,4 +47,4 @@ readLobstersRss.__interval__ = 5000;

module.exports = [
readLobstersRss,
];
];