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

Structure 2 #111

Open
chiragsalian opened this issue Mar 25, 2020 · 12 comments
Open

Structure 2 #111

chiragsalian opened this issue Mar 25, 2020 · 12 comments

Comments

@chiragsalian
Copy link
Owner

Assume PR

@chiragsalian
Copy link
Owner Author

Testing this today. Some thoughts,

  1. Tests -> Step 2: I don't quite follow how commenting with a dev chat link on an Expensify issue would help. Unless you are not testing dev chats and that would be weird since this is testable on dev.

@chiragsalian
Copy link
Owner Author

  1. Tests -> Step 5:
    a. "comment out lines 149-152" - thats linked to a diff of files.
    b. "replace line 154" if this is the line with "Github_API::LABEL_CONCIERGE_DONT_REOPEN" then we dont really need to comment this out since most issues don't have this label by default.
    c. "line 148 with" also goes to a diff so im not sure what to look at
    d. Similar comment for the other links.
    Remember to use permalink in your links i.e., open the file to see its entire contents, go the line, right click on the side and select permalink. This way the link doesn't change with commits,
    image

@chiragsalian
Copy link
Owner Author

  1. Tests -> Step 2,3,4,5,6: These can be consolidated for easier steps on dev. How i do the dev testing which i did today as well,
    a. Create your own repository. Go to settings -> webhook and add your webhook. set content-type to application/json and select "send me everything".
    b. Go to Web-Expensify/vendor/expensify/PHP-Libs/src/Github/API.php and update REPO_OWNER and ISSUE_REPO to your githubname and the new repo. For me its,
const REPO_OWNER = 'chiragsalian';
const ISSUE_REPO = 'javascriptExercises';

(at this point github/webhook.php will work with issues in your new repo)

@chiragsalian
Copy link
Owner Author

  1. Tests -> Step 7. So i got this,
    image
    i don't see any options. I bet this because i dont have [email protected] as an account. It might need to be mentioned in the test steps to have this account on dev. Seeing it now,
    image
    Can we update the UI to not show this when email is [email protected]?
    image

@chiragsalian
Copy link
Owner Author

  1. Let's hyperlink the link,
    image

@chiragsalian
Copy link
Owner Author

  1. I don't think we should show the following icons since they don't make much sense in this chat: github_issue, escalate (maybe will this work in another tier?), supportal, and intercom.

@chiragsalian
Copy link
Owner Author

To answer comment
-> No longer relevant after updating the instructions following 3.

@chiragsalian
Copy link
Owner Author

To answer comment
-> No longer relevant after updating the instructions following 3, but thanks for the tip!

@chiragsalian
Copy link
Owner Author

To answer comment,
-> Yep, thats WAY better. Updated. Thanks!

@chiragsalian
Copy link
Owner Author

To answer comment,
-> Added to the instructions. I don't think we need to hide that info for [email protected]. It is a closed account in PROD, so it is hidden by default. (You can just close it in dev to mimic the same behavior).

@chiragsalian
Copy link
Owner Author

To answer comment,
-> Sure thing

@chiragsalian
Copy link
Owner Author

To answer comment,
-> Makes sense

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant