Skip to content

Conversation

@erezrokah
Copy link
Member

Summary

Part of https://github.com/cloudquery/cloudquery-issues/issues/2034 needs cloudquery/cloudquery-api-go#205


Use the following steps to ensure your PR is ready to be reviewed

  • Read the contribution guidelines 🧑‍🎓
  • Run go fmt to format your code 🖊
  • Lint your changes via golangci-lint run 🚨 (install golangci-lint here)
  • Update or add tests 🧪
  • Ensure the status checks below are successful ✅

@github-actions github-actions bot added the feat label Jul 29, 2024
@github-actions
Copy link

github-actions bot commented Jul 29, 2024

⏱️ Benchmark results

Comparing with 9dac994

  • Glob-8 ns/op: 91.69 ⬇️ 1.56% decrease vs. 9dac994

@erezrokah erezrokah marked this pull request as ready for review July 30, 2024 09:29
@erezrokah erezrokah requested review from a team, bbernays and marianogappa and removed request for bbernays July 30, 2024 09:29
Copy link
Contributor

@marianogappa marianogappa left a comment

Choose a reason for hiding this comment

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

Nothing prevents two different implementors from documenting the same cloud permissions differently 👀 it's just a string. I think it's fine, since it's just documentation.

{Name: "incremental_key", Type: arrow.BinaryTypes.String, IncrementalKey: true},
{Name: "multiple_attributes", Type: arrow.BinaryTypes.String, PrimaryKey: true, IncrementalKey: true, NotNull: true, Unique: true},
},
PermissionsNeeded: []string{"storage.buckets.list"},
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess the only other feature than a literal documentation string is the fact that commas are a special character; do you want to check that the splitting is maintained when you go back and forth?

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah good point let me handle that

@marianogappa marianogappa self-requested a review July 30, 2024 09:58
@kodiakhq kodiakhq bot merged commit 863b906 into main Jul 30, 2024
@kodiakhq kodiakhq bot deleted the feat/add_ability_to_document_permissions branch July 30, 2024 10:03
kodiakhq bot pushed a commit that referenced this pull request Jul 30, 2024
kodiakhq bot pushed a commit that referenced this pull request Jul 30, 2024
🤖 I have created a release *beep* *boop*
---


## [4.55.0](v4.54.0...v4.55.0) (2024-07-30)


### Features

* Add `PermissionsNeeded` to tables schema ([#1827](#1827)) ([863b906](863b906))


### Bug Fixes

* Handle commas in permissions array ([#1835](#1835)) ([b633aed](b633aed))

---
This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
kodiakhq bot pushed a commit to cloudquery/cloudquery that referenced this pull request Aug 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants