Skip to content

Conversation

@dlouzan
Copy link
Collaborator

@dlouzan dlouzan commented Aug 8, 2018

  • use the new gitlab 11.2 min_access_level parameter in the groups api:
    publish rights are now configurable based on the access level of the
    user to the groups, it doesn't require owner permissions
  • add new configuration parameters for access and publish access
    levels

Fixes #5

- use the new gitlab 11.2 `min_access_level` parameter in the groups api:
  publish rights are now configurable based on the access level of the
  user to the groups, it doesn't require owner permissions
- add new configuration parameters for `access` and `publish` access
  levels
@bufferoverflow
Copy link
Owner

Great! I guess we need the following before we merge:

  • compatible with <11.2 (via API call or legacy_mode: true for old versions < 11.2)
  • remove access cache, so just one instead of two api calls to gitlab
  • create a upstream issue for access and publish group

@bufferoverflow bufferoverflow added this to the 2.0.0 milestone Aug 9, 2018
@dlouzan
Copy link
Collaborator Author

dlouzan commented Aug 9, 2018

Quick question: if we are going to add the legacy flag in the configuration, which will be disabled by default, I guess we can also set the default publish access level to $maintainer? At least it feels more natural to me. When legacy_mode: true, the value applied will be $owner anyway, there's no other option pre-11.2.

@bufferoverflow
Copy link
Owner

Good idea @dlouzan , fully agree!

- remove `access` access level configuration parameter since it cannot be
  used with the current plugins api
- support gitlab 9.0+ with a new `legacy_mode` configuration flag; when
  not in legacy mode, gitlab 11.2+ is required
- improved README, document all configuration options
@dlouzan dlouzan force-pushed the feat/gitlab-11.2-user-api-improvements branch from c6be5e7 to 68068bf Compare August 9, 2018 18:22
@dlouzan
Copy link
Collaborator Author

dlouzan commented Aug 9, 2018

Just pushed the latest changes:

  • remove access access level configuration parameter since it cannot be used with the current plugins api
  • support gitlab 9.0+ with a new legacy_mode configuration flag; when not in legacy mode, gitlab 11.2+ is required
  • improved README, document all configuration options

README.md Outdated
## Gitlab Version Compatibility

- If `legacy_mode: false`: Gitlab 11.2+
Copy link
Owner

Choose a reason for hiding this comment

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

Please write false or not defined here

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's indicated below in the configuration options list that legacy_mode: false is the default, but I will make it also explicit here.

@@ -0,0 +1,36 @@
storage: /tmp/storage/data
Copy link
Owner

Choose a reason for hiding this comment

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

this file is nearly the same as conf/docker.yml , why do we need this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ooops, that's for my local testing, shouldn't be there

- remove spurious localhost tests configuration file
- misc corrections in README
@dlouzan dlouzan force-pushed the feat/gitlab-11.2-user-api-improvements branch from 89bdcbf to 207a490 Compare August 10, 2018 10:18
@dlouzan
Copy link
Collaborator Author

dlouzan commented Aug 10, 2018

Done, I'm adding commits on top of the original PR commit. Feel free to squash when accepting.

@bufferoverflow
Copy link
Owner

Great work!

@bufferoverflow bufferoverflow merged commit 323ef0b into master Aug 10, 2018
@dlouzan dlouzan deleted the feat/gitlab-11.2-user-api-improvements branch August 10, 2018 12:12
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

Successfully merging this pull request may close these issues.

3 participants