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

[docs-only] Add release guides for accounts & settings, fix issue link in settings/package.json #1952

Merged
merged 6 commits into from
Apr 28, 2021

Conversation

pascalwengerter
Copy link
Contributor

Description

Within the v1.5.0 release of oCIS, I had to work on the accounts and settings extensions and almost forgot adding their respective embed.go files to Git. After searching for a while and requesting help from @fschade I managed to do so, but documenting this in written form will help both outside contributors as well as team members in the future.

Types of changes

  • Documentation enhancement

@update-docs
Copy link

update-docs bot commented Apr 21, 2021

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

@pascalwengerter pascalwengerter requested review from refs and fschade April 21, 2021 23:21
#### Updating ocis-accounts

1. Make sure you are inside the [ocis repository](https://github.com/owncloud/ocis) and on your feature branch
2. Change into accounts' asset package folder via `cd accounts/pkg/assets`
Copy link
Contributor

Choose a reason for hiding this comment

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

running 'make generate' in the root of the oCIS repository generates these assets for all oCIS extension and you also can invoke it inside the folder of an extension to just run in for a single extension.

It is already described here, but it seems to be not accessible enough / in an place where people look for it: https://owncloud.dev/ocis/development/build/#build-the-ocis-binary

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should we keep both texts since both are (technically) correct? I wrote those with the idea in mind that somebody only touches the accounts/settings extension and finds all the necessary information in one place :)

Copy link
Contributor

Choose a reason for hiding this comment

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

I see it as a very good addition but would like to advice people only to use 'make generate' because it acts as abstraction an d also does more than just 'go generate'.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated the PR with a suggestion!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay @wkloucek I just ran make generate on master and ended up with five embed.go files that vary from the ones in version control 🥴 and now I'm confused!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@wkloucek updated the guides and reduced them to the make generate cmd

@refs
Copy link
Member

refs commented Apr 27, 2021

My dream is to use the new embed package and replace this asset generation :)

docs/extensions/accounts/releasing.md Outdated Show resolved Hide resolved
docs/extensions/accounts/releasing.md Show resolved Hide resolved
@pascalwengerter
Copy link
Contributor Author

My dream is to use the new embed package and replace this asset generation :)

This cries for a ticket in the oCIS repo so it can be added to a sprint and implemented soon? 🕴🏽

@pascalwengerter pascalwengerter force-pushed the 22042021_accounts-settings-generate-docs branch from efd1741 to 95bf02f Compare April 27, 2021 16:16
@refs
Copy link
Member

refs commented Apr 27, 2021

My dream is to use the new embed package and replace this asset generation :)

This cries for a ticket in the oCIS repo so it can be added to a sprint and implemented soon? 🕴🏽

#1199 ^^

@individual-it individual-it removed their request for review April 28, 2021 03:18
@wkloucek
Copy link
Contributor

My dream is to use the new embed package and replace this asset generation :)

This cries for a ticket in the oCIS repo so it can be added to a sprint and implemented soon? 🕴🏽

#1199 ^^

If we document only 'make generate' this change is abstracted for users and they can continue using their known command, so this should not block this PR ;-)

@refs
Copy link
Member

refs commented Apr 28, 2021

My dream is to use the new embed package and replace this asset generation :)

This cries for a ticket in the oCIS repo so it can be added to a sprint and implemented soon? 🕴🏽

#1199 ^^

If we document only 'make generate' this change is abstracted for users and they can continue using their known command, so this should not block this PR ;-)

exactly, it's a "behind the curtains" kind of change

docs/extensions/accounts/releasing.md Outdated Show resolved Hide resolved
docs/extensions/settings/releasing.md Outdated Show resolved Hide resolved
Pascal Wengerter and others added 2 commits April 28, 2021 11:55
@phil-davis phil-davis self-requested a review April 28, 2021 11:06
Copy link
Contributor

@wkloucek wkloucek left a comment

Choose a reason for hiding this comment

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

LGTM

@pascalwengerter pascalwengerter merged commit a33c81c into master Apr 28, 2021
@delete-merged-branch delete-merged-branch bot deleted the 22042021_accounts-settings-generate-docs branch April 28, 2021 12:03
ownclouders pushed a commit that referenced this pull request Apr 28, 2021
Merge: 8ec196d 46db2e8
Author: Pascal Wengerter <[email protected]>
Date:   Wed Apr 28 13:02:59 2021 +0100

    Merge pull request #1952 from owncloud/22042021_accounts-settings-generate-docs

    [docs-only] Add release guides for accounts & settings, fix issue link in settings/package.json
@fschade fschade mentioned this pull request Apr 28, 2021
12 tasks
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.

4 participants