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

Migrate to new organization #141

Merged
merged 42 commits into from
Oct 24, 2022
Merged

Migrate to new organization #141

merged 42 commits into from
Oct 24, 2022

Conversation

Simply007
Copy link
Contributor

Motivation

Which issue does this fix? Fixes #issue number

If no issue exists, what is the fix or new feature? Were there any reasons to fix/implement things that are not obvious?

Checklist

  • Code follows coding conventions held in this repo
  • Automated tests have been added
  • Tests are passing
  • Docs have been updated (if applicable)
  • Temporary settings (e.g. variables used during development and testing) have been reverted to defaults

How to test

If manual testing is required, what are the steps?

@Simply007 Simply007 marked this pull request as ready for review October 4, 2022 05:09
@IvanKiral IvanKiral self-assigned this Oct 13, 2022
@IvanKiral
Copy link
Contributor

IvanKiral commented Oct 13, 2022

DELIVERY SDK:

  • There are some Kontent instead of Kontent.ai ( comment suggestions in at least Readme are coming later with full review)
  • All .java files does not have updated MIT license on the top of the file
  • Missing Javadocs (AsyncCache, Delivery client) - add or maybe create Issue?
  • Restructure folders -> sometimes it is hard to implementation of interface (optional, also candidate for new issue)
  • Code is mostly good from my perspective, some comments are coming with full review later

@IvanKiral
Copy link
Contributor

IvanKiral commented Oct 17, 2022

Delivery-sdk-generators:

  • Code looks good, only there are some only Kontent mentions. Although there are only comments they can be changed with replace

@IvanKiral
Copy link
Contributor

IvanKiral commented Oct 17, 2022

sample-app-android:

  • There is a Floating Action button registered in ArticlesFragment.java it is not visible in the app though.
  • There are some not changed Kontent -> Kontent.ai

README.md Outdated Show resolved Hide resolved
delivery-sdk/README.md Outdated Show resolved Hide resolved
delivery-sdk/README.md Outdated Show resolved Hide resolved
delivery-sdk/README.md Outdated Show resolved Hide resolved
delivery-sdk/README.md Outdated Show resolved Hide resolved
@@ -22,7 +22,7 @@
* SOFTWARE.
*/

package kentico.kontent.delivery;
package kontent.ai.delivery;

import com.fasterxml.jackson.annotation.JsonProperty;

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe lombok for getters? or is it this way for better code readability?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would leave it this way - or create a separate issue if you like it more 👍🏼

sample-app-android/src/main/res/values/strings.xml Outdated Show resolved Hide resolved
Simply007 and others added 8 commits October 18, 2022 18:58
Co-authored-by: Ivan Kiráľ <54802833+IvanKiral@users.noreply.github.com>
Co-authored-by: Ivan Kiráľ <54802833+IvanKiral@users.noreply.github.com>
Co-authored-by: Ivan Kiráľ <54802833+IvanKiral@users.noreply.github.com>
Co-authored-by: Ivan Kiráľ <54802833+IvanKiral@users.noreply.github.com>
@Simply007
Copy link
Contributor Author

Simply007 commented Oct 20, 2022

DELIVERY SDK:

  • There are some Kontent instead of Kontent.ai ( comment suggestions in at least Readme are coming later with full review)
  • All .java files does not have updated MIT license on the top of the file

I haven't found any old licences

  • Missing Javadocs (AsyncCache, Delivery client) - add or maybe create Issue?

Added for asynccache and delivery client - create issue for the rest

  • Restructure folders -> sometimes it is hard to implementation of interface (optional, also candidate for new issue)

Create an issue if you want to

  • Code is mostly good from my perspective, some comments are coming with full review later

@Simply007
Copy link
Contributor Author

Simply007 commented Oct 20, 2022

sample-app-spring-boot:

  • Change fonts to work sans
  • Article.html has twice

I don't understand

  • Date in articles has weird format
  • Linked items in rich text element are not working.. In article coffee berages explained clicking on so rich as today should navigate us to the which brewing fits you? article

@Simply007 Simply007 assigned Simply007 and unassigned IvanKiral Oct 24, 2022
@Simply007 Simply007 merged commit e20c4d7 into master Oct 24, 2022
@Simply007 Simply007 deleted the migration branch October 24, 2022 13:02
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.

None yet

2 participants