Skip to content
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.

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

Change the occourences of discordapp.com to discord.com #555

Merged
merged 1 commit into from
May 9, 2020

Conversation

Joshix-1
Copy link
Member

@Joshix-1 Joshix-1 commented May 4, 2020

see #554!
I changed everything I could find. I don't know if it is complete.

Copy link

@CharlesLgn CharlesLgn left a comment

Choose a reason for hiding this comment

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

I think you forgot some.
When I write discordapp in my IDE, I found 53 occuration

Maybe a good solution will be to store the main url (discord.com) in a properties. load it on the begin of an app anreplace all real occuration (not javadoc) by this value

@Joshix-1
Copy link
Member Author

Joshix-1 commented May 5, 2020

I read somewhere, that the cdn.discordapp.com won't change.

@Bastian
Copy link
Member

Bastian commented May 5, 2020

image
You missed one in the mockserver.properties file.

@Bastian
Copy link
Member

Bastian commented May 5, 2020

I read somewhere, that the cdn.discordapp.com won't change.

Indeed. Here's the reason:
image

@Bastian
Copy link
Member

Bastian commented May 5, 2020

Maybe a good solution will be to store the main url (discord.com) in a properties. load it on the begin of an app anreplace all real occuration (not javadoc) by this value

Agreed. Even though I think that it should be enough to have it as a public static final String constant somewhere:

public static final String DISCORD_DOMAIN = "discord.com";
public static final String DISCORD_CDN_DOMAIN = "cnd.discordapp.com";

Maybe even in the API package in the DiscordApi interface?

@Bastian Bastian added the high priority An issue or pull request with a high priority label May 5, 2020
@Bastian Bastian linked an issue May 5, 2020 that may be closed by this pull request
@Vampire
Copy link
Member

Vampire commented May 5, 2020

You missed one in the mockserver.properties file.

Just have both in there.
It is for the generated SSL certificate for MockServer based tests.
Doesn't hurt to simply have both in there.

@@ -12,7 +12,7 @@
* The base link of a bot invite.
*/
public static final String BASE_LINK =
"https://discordapp.com/oauth2/authorize?client_id=%s&scope=bot&permissions=%s";
"https://discord.com/oauth2/authorize?client_id=%s&scope=bot&permissions=%s";

Choose a reason for hiding this comment

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

"https://"+ DISCORD_URL +"/oauth2/authorize?client_id=%s&scope=bot&permissions=%s"
or using String.formatMessage

@@ -698,7 +698,7 @@ default String getReadableContent() {
*/
default URL getLink() throws AssertionError {
try {
return new URL("https://discordapp.com/channels/"
return new URL("https://discord.com/channels/"

Choose a reason for hiding this comment

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

"https://"+ DISCORD_URL +"/channels/"
or using String.formatMessage

Pattern.compile("(?x) # enable comment mode \n"
+ "(?i) # ignore case \n"
+ "(?:https?+://)?+ # 'https://' or 'http://' or '' \n"
+ "discord(?:app)?\\.com/channels/ # 'discord(app).com/channels/' \n"

Choose a reason for hiding this comment

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

why adding (?:app)?\\ the url will only be discord.com/channels/ no ?

Copy link
Member Author

@Joshix-1 Joshix-1 May 5, 2020

Choose a reason for hiding this comment

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

Currently both is possible. And I would remove the (?:app)? only when only discord.com/channels is possible.

Copy link
Member

Choose a reason for hiding this comment

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

I'd also keep it, someone could post an old link. But add + after ?, there is no need to backtrack.

Copy link
Member

Choose a reason for hiding this comment

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

And now the alignment again. :-)

@Bastian
Copy link
Member

Bastian commented May 8, 2020

@Joshix-1 Do you plan to add the constants in this PR?

I would be ok if you don't. In this case I would create an issue for it and someone else (maybe @CharlesLgn 😉 ) can create a separate PR for it.

@@ -49,6 +49,8 @@
* This class is the most important class for your bot, containing all important methods, like registering listener.
*/
public interface DiscordApi extends GloballyAttachableListenerManager {
public static final String DISCORD_DOMAIN = "discord.com";
public static final String DISCORD_CDN_DOMAIN = "cnd.discordapp.com";
Copy link
Contributor

Choose a reason for hiding this comment

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

cnd != cdn

Copy link
Member

@Bastian Bastian left a comment

Choose a reason for hiding this comment

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

Like @CharlesLgn said, the constants should also be used.

@@ -49,6 +49,8 @@
* This class is the most important class for your bot, containing all important methods, like registering listener.
*/
public interface DiscordApi extends GloballyAttachableListenerManager {
public static final String DISCORD_DOMAIN = "discord.com";
public static final String DISCORD_CDN_DOMAIN = "cnd.discordapp.com";
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
public static final String DISCORD_CDN_DOMAIN = "cnd.discordapp.com";
/*
* The domain of Discord's CDN.
*/
public static final String DISCORD_CDN_DOMAIN = "cnd.discordapp.com";

@@ -12,7 +12,7 @@
* The base link of a bot invite.
*/
public static final String BASE_LINK =
"https://discordapp.com/oauth2/authorize?client_id=%s&scope=bot&permissions=%s";
"https://discord.com/oauth2/authorize?client_id=%s&scope=bot&permissions=%s";
Copy link
Member

Choose a reason for hiding this comment

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

When we have the constants, we should also use them.

Suggested change
"https://discord.com/oauth2/authorize?client_id=%s&scope=bot&permissions=%s";
"https://" + DiscordApi.DISCORD_DOMAIN + "/oauth2/authorize?client_id=%s&scope=bot&permissions=%s";

@@ -698,7 +698,7 @@ default String getReadableContent() {
*/
default URL getLink() throws AssertionError {
try {
return new URL("https://discordapp.com/channels/"
return new URL("https://discord.com/channels/"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
return new URL("https://discord.com/channels/"
return new URL("https://" + DiscordApi.DISCORD_DOMAIN + "/channels/"

@@ -153,7 +153,7 @@ public void setGlobal(boolean global) {
* @return The full url of the endpoint.
*/
public String getFullUrl(String... parameters) {
StringBuilder url = new StringBuilder("https://discordapp.com/api/v" + Javacord.DISCORD_API_VERSION + getEndpointUrl());
StringBuilder url = new StringBuilder("https://discord.com/api/v" + Javacord.DISCORD_API_VERSION + getEndpointUrl());
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
StringBuilder url = new StringBuilder("https://discord.com/api/v" + Javacord.DISCORD_API_VERSION + getEndpointUrl());
StringBuilder url = new StringBuilder("https://" + DiscordApi.DISCORD_DOMAIN + "/api/v" + Javacord.DISCORD_API_VERSION + getEndpointUrl());

@Bastian Bastian force-pushed the patch-1 branch 3 times, most recently from 71c1d4b to 8b3d540 Compare May 9, 2020 18:12
@Bastian Bastian added this to the Next Version milestone May 9, 2020
This commit updates from the old discordapp.com domain to the new
discord.com domain. It also introduces constants for the Discord
domain and Discord's CDN domain.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
high priority An issue or pull request with a high priority
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Change endpoints from discordapp.com to discord.com
5 participants