tobous on build
[DOC] Adjust documentation for … (compare)
@Flowdalic Uff, its been a while. I think the smack stuff just got lost in the chaos. :sweat_smile: But we are still very much interested in updating smack. We have been working hard on updating our outdated libraries along with our build infrastructure and smack is one of the big ones, especially since it provides a central functionality of the plugin and is in a somewhat messy state with all of the patch stuff we added. So we really appreciate your efforts.
I had a short look at your branch but didn't really get far. (But I am also not that well-versed in the network part of Saros, so that doesn't really mean that much :wink:) As @srossbach already mentioned, formatting the code with our formatter (google java format 1.6) reduces the PR size considerably (from +3300 -4800 LOC to +1000 LOC - 2700 LOC, which is still a lot, but more manageable).
The patch seems like a WIP (judging from the compiler errors). Are you still interested in working on the migration? If so, what are your general plans? Maybe we can work out some sort of cooperation to make the migration easier. :smile: Do you have a general estimate on how much work still is necessary to complete the migration?
Extending User Permissions
My job is to come up with an extension of the User Permissions mechanism. I've seen there is already a somewhat flexible Permission implemented in the User Class:
My question: What would you like specifically to be extended? Are there use cases for currently missing functionality? The only problem i could find until now is the missing Permission for Users to upload documents into the Session. From my point of view it would be sufficient to add a specifier to the enum Permissions and an instance method like hasShareDocumentPermission()..?
Hi, @stefaus, @Drakulix and me already talked a bit about your topic (Adding a permission model that will improve the server workflow). I'll post a summary of our talk below. @Memo2109 I think you should concentrate on the first step mentioned in this summary. @srossbach what do you think?
The current server just accepts all requests of users to join the session.
Therefore it is necessary to think about a permission model.
Using different permission models for server sessions and common session (without a server) is unnecessary. Therefore, it is necessary to create a permission model that allows to implement the current user permissions (the permissions of the host user and other users).
@Memo2109 Hey Mehmet, normaly please create a (draft) pull request to get feedback for your code.
I think instead of creating a separate
UserPrivilege class, using a
EnumSet would be a simpler approach. What you have to keep in mind is also concurrency. Using a
HashMap will not work here (neither
EnumSet without additional handling) and instead of a separate method to check Privileges, this should stick to a single get method like your
About the Privileges, a User is created inside a Session, thats why handling Permissions like to
Start a Session Server here will not work.
Once saros-project/saros#752 is merged, I would like to do another round of testing for the vector time patch saros-project/saros#714. As I was not able to produce any issues in a two person setup (even with concurrent activities), a multi-user test is quite important (especially with the weird issues that occurred the last time).
@m273d15 @stefaus @srossbach What does your availability look like in next week? My initial suggestion would be Monday or Tuesday afternoon, but I don't have any set events/meetings on Monday, Tuesday, or Thursday, so I am relatively open timewise. Wednesday before 17:00 would be fine as well.
I have one question, as I make my last PR I have seen that tests startet like codacy and travis ci. My question is should I also make the codacy and travis ci tests local on my repo befor I make a PR ? If yes have you an introduction for setting up codacy and travis ci. Thank you very much.
@Memo2109 No, you don't have to add the checks to your local fork of the repository. As you already pointed out, they are always run when you create a PR onto the main repo.
I changed the description of the
Area: X issue labels to state "Issues affecting X". In my opinion, this makes the tags much more usable as they then provide a good tool for getting an overview which PRs affect which area of the code. In my opinion, the current restrictions of having them be "specific to the area" makes sense for issues but not for PRs, as they often bundle multiple commit dealing with different parts of an issue.
Anyone have any issues with this change?
12.12. at 16:30 isn't ideal for me. I have a meeting at 15:15 for which I don't know how long it is going to take. I could be done by 15:45 but it could easily also take until 17:00 (or longer), depending on my waiting time.
For me, the following dates would work better:
Or, if don't find any other date/time, you could keep the session on the 12.12. at 16:30 (or 17:00) and I will join in as soon as I can.
What a pity! Sure, I will send you an email.
Do you want to announce some annotation fixes with the new release while there are still issues with them ?
Do you talk about the issue #32 ?
I just noticed that the "Terminate Session" icon (the stop button; adjusted leave session icon on host side) does not get reset to the default "lease session" icon (the door) on session end. This isn't strictly an issue as the icon gets set correctly when a new session is started as a client, but it still seems weird to me as it leads to an inconsistent UI in the disconnected state.
My inclination would be to create an issue for this and mark it as "Prio: Low" and "Help Wanted" & "good first issue" as it is not strictly necessary but nice to have to be consistent.
Any input on this?
Hi, we have multiple stale PRs that are not updated in the last two months.
I would like to avoid a graveyard of old PRs. Therefore, could we clarify which PRs "can be closed" (if so, please close the PR and remove the branch), "is in review" (if so, we should complete the review),...
If you want to preserve the commits, because you want to use the code in the future, please close the PR without deleting the branch.
The run was started after rebasing the PR and doing some minor changes that should not be able to influence the result. The previous STF run and the STF run on the base branch I rebased on both succeeded and the test runs successfully in my local environment.