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

Removing minting capabilities of Short Chat view #18

Merged
merged 3 commits into from
Sep 23, 2019

Conversation

megoth
Copy link
Contributor

@megoth megoth commented Sep 16, 2019

As part of #17

Partial fix for #16

@megoth megoth requested a review from timbl September 16, 2019 16:52
@megoth megoth added the release-major Add to PR to indicate that merging it denotes a major (semver) release label Sep 16, 2019
@megoth
Copy link
Contributor Author

megoth commented Sep 16, 2019

Marking this as a major change (in semver) to this package, as the change is not backward-compatible.

megoth added a commit to SolidOS/solid-panes that referenced this pull request Sep 16, 2019
This is dependent on the changes in SolidOS/chat-pane#18, so do not merge until package.json reflects this update.

Partial fix for SolidOS/chat-pane#16
Copy link

@Vinnl Vinnl left a comment

Choose a reason for hiding this comment

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

I'd prefer it if you just removed the old code (though preserve the comment on why you can't mint a new Short Chat) - it can always be retrieved from Git anyway. Leaving it in makes it uncertain for future developers whether the code is still needed or not, and makes genuine comments and code harder to parse (for the human).

@megoth
Copy link
Contributor Author

megoth commented Sep 17, 2019

@Vinnl Have implemented your requested change ^_^

Copy link
Contributor

@timbl timbl left a comment

Choose a reason for hiding this comment

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

Sorry, we need to leave the code in because it used by the meetulator.
We also may use minting of short chats as threads within a long chat, like threads in Slack.
We need to instead have separate filter for minting as for viewing, it seems ... creationAudience or something

@megoth
Copy link
Contributor Author

megoth commented Sep 17, 2019

Sorry, we need to leave the code in because it used by the meetulator.
We also may use minting of short chats as threads within a long chat, like threads in Slack.
We need to instead have separate filter for minting as for viewing, it seems ... creationAudience or something

@timbl and I had a talk outside of GH, and concluded that we will add Power User to the list of audience for Short Chat view, which will limit the creation of related resources to Power Users.

@megoth megoth added release-minor Add to PR to indicate that merging it denotes a minor (semver) release and removed release-major Add to PR to indicate that merging it denotes a major (semver) release labels Sep 17, 2019
@megoth
Copy link
Contributor Author

megoth commented Sep 17, 2019

@timbl I've done the changes we've talked about, please review when you have time.

@megoth megoth merged commit 0a5e21e into master Sep 23, 2019
@megoth megoth deleted the remove-minting-for-small branch September 23, 2019 10:50
@brownhoward brownhoward added this to the Data Browser Release 3 milestone Oct 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-minor Add to PR to indicate that merging it denotes a minor (semver) release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants