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

3.0.0 Major Update #36

Open
wants to merge 76 commits into
base: master
Choose a base branch
from
Open

3.0.0 Major Update #36

wants to merge 76 commits into from

Conversation

moelrobi
Copy link
Member

@moelrobi moelrobi commented Jul 2, 2018

Major Update PR for AminoJS

@moelrobi moelrobi added enhancement New feature or request MajorUpdate All PR's / Issues that are with the latest major Update. MajorUpdatePR All PR's / Issues that indicate the Next Major Update labels Jul 2, 2018
@akatsukilevi
Copy link
Contributor

Hey, any update for merging 3.0.0 to master?

@moelrobi
Copy link
Member Author

moelrobi commented Jan 4, 2019

Once all checks are passing and all methods and endpoints have unit testing, then it will merge into Master. (Prob. Today)

akatsukilevi and others added 3 commits January 4, 2019 08:54
Travis CI is throwing a ``Unknown option "setTimeout" with value 7000 was found. This is probably a typing mistake. Fixing it will remove this message.``exception. Fixed it here

Also had to create a new branch because i edited it directly on GitHub(no ide ready here)
Travis CI is throwing a ``Unknown option "setTimeout" with value 7000 was found. This is probably a typing mistake. Fixing it will remove this message.``exception. Fixed it here

Also had to create a new branch because i edited it directly on GitHub(no ide ready here)
Note to myself: never Debug again drunk, with only a phone. thanks levi. 👍
@moelrobi
Copy link
Member Author

moelrobi commented Jan 4, 2019

Hey @akatsukilevi,
since i was in the middle of creating tests for the endpoints i stumbbeld upon your getCheckInCommunities Module.

There is this documentation:

/**
 * Loads Favorite Members
 * @param {SecurityString} sid For authenticating with the Narvii-API.
 * @param {String} coms A list of communities to get the check-in status, separated by "&"
 * @param {String} timezone The desired timezone to check(It will affect the user Check-In Functionality)
 * @returns {Object} Object with all the Check In info of the communities specified from the Logged-in User has in an Array.
 */

Couldn't it be easier if the List of communites would be spearated with , and not by &? Why this design choice?

EDIT: Also please keep the Top Documentation clean. I dont think this Module loads favorite Members. >-> xD

@akatsukilevi
Copy link
Contributor

akatsukilevi commented Mar 18, 2019

Hey @akatsukilevi,
since i was in the middle of creating tests for the endpoints i stumbbeld upon your getCheckInCommunities Module.

There is this documentation:

/**
 * Loads Favorite Members
 * @param {SecurityString} sid For authenticating with the Narvii-API.
 * @param {String} coms A list of communities to get the check-in status, separated by "&"
 * @param {String} timezone The desired timezone to check(It will affect the user Check-In Functionality)
 * @returns {Object} Object with all the Check In info of the communities specified from the Logged-in User has in an Array.
 */

Couldn't it be easier if the List of communites would be spearated with , and not by &? Why this design choice?

EDIT: Also please keep the Top Documentation clean. I dont think this Module loads favorite Members. >-> xD

First sorry for take ages to reply XD
Also, it is separated by & instead of , due to the Amino API itself. I can make it be separated by ,, and before send substitute by &, so the API won't fail and the design will get better
What do you think?

EDIT: LOL i didn't noticed that there! XD I'm fixing the Top Documentation now

Copy link
Contributor

@akatsukilevi akatsukilevi left a comment

Choose a reason for hiding this comment

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

Despite the issue Community Global Config System(#51) Not being finished, it added a lot of things to the repo

@akatsukilevi
Copy link
Contributor

Hey, how's the situation for this pull request?

Added fix to send the image type argument through Amino
@akatsukilevi
Copy link
Contributor

Hey, any update for this merge?

@akatsukilevi akatsukilevi self-requested a review June 25, 2019 14:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request MajorUpdate All PR's / Issues that are with the latest major Update. MajorUpdatePR All PR's / Issues that indicate the Next Major Update
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants