[flutter_local_notifications] android Initialization defaultIcon can null

[jawa0919]

if defaultIcon is null, usesgetApplicationInfo().icon

As this repository hosts two packages, please ensure the PR title starts with the name of the package that it relates to using square brackets (e.g. [flutter_local_notifications]). The contribution guidelines can be found at https://github.com/MaikuB/flutter_local_notifications/blob/master/CONTRIBUTING.md. Please review this as it contains details on what to follow when submitting a PR.

[MaikuB]

Thanks for the PR. Can you be more specific on what this PR is attempting to fix? The defaultIcon should be nullable when it comes to the AndroidInitializationSettings class and was missed so that's a good catch. However, the reason it's meant to be nullable so that developers specify a default icon or an icon per notification. Having a fallback (i.e. getApplicationInfo().icon) isn't suitable as it won't help catch the scenario where a developer has forgotten to specify either a default icon or an icon for the notification itself. As such, I would that logic should be removed.

The other thing is whilst the positional optional parameter avoids a breaking change, from what I've gathered the convention within the Flutter/Dart community seems to be to used a named parameter. Perhaps this would be the better approach? There's a 10.0 pre-release with a branch for it so that would allow for a breaking change like this. Alternatively, using a positional parameter could be used for 9.x but then 10.x changes it to be named. Thoughts?

[jawa0919]

@MaikuB

My project is a multi-flavor project, and I need to set notification icons for each flavor, even though it's the same as their appIcon. So I just submitted this pr. I think setting an android resource on flutter is a bit risky. This defaultIcon is the name of the Android resource, not even the full name. The ideal situation would be to set the notification icon in flttet/assets, but this has a lot of difficulties.

Unable to locate the error reported. For those who are proficient in Android development, it is just a simple question. Some mobile developers may not be Android developers before developing Flutter. For novice ios developers, to use this plugin, they may need to be familiar with Android resource path configuration.

Let the beginners use it out of the box, and let the proficient research it by themselves

I'm using positional optional parameters on purpose, it's optimal for 9.0, it doesn't require changes to legacy code. If this configuration is used in the new version, I will commit a pr in the new branch with the named optional parameter.

Thanks for review

---original---

我的项目是一个多风味的项目，我需要设置每一个风味的通知图标，尽管和他们的appIcon是一致。所以我才提交这个pr。我觉得在flutter上设置一个Android资源有一点的风险。这个defaultIcon是Android的资源的名称，甚至不是全名。最理想的情况应该是在fluttet/assets中设置通知图标，但是这个有很多困难。

无法定位报错的问题。精通Android开发的人来说者只是一个简单问题，一些移动开发者在开发Flutter之前可能不是Android开发者，对于新手ios开发者来说，使用这个插件，可能需要先熟悉Android的资源路径配置。

让初学者开箱即用，让精通者自己研究

我是有意使用位置可选参数的，对于9.0来说是最优的，它不需要更改旧代码。如果在新版本中使用上这个配置，我会在新分支中使用命名可选参数提交一个pr。

谢谢审核

[MaikuB]

My project is a multi-flavor project, and I need to set notification icons for each flavor, even though it's the same as their appIcon. So I just submitted this pr.

Fair enough though in this case that means it's something specific to your scenario so with that in mind would you be able to remove the fallback logic?

I think setting an android resource on flutter is a bit risky. This defaultIcon is the name of the Android resource, not even the full name. The ideal situation would be to set the notification icon in flttet/assets, but this has a lot of difficulties.

Unable to locate the error reported. For those who are proficient in Android development, it is just a simple question. Some mobile developers may not be Android developers before developing Flutter. For novice ios developers, to use this plugin, they may need to be familiar with Android resource path configuration.

I'm not sure what point you were trying to make here but it seems like you're neglecting that a reminder the fact that plugins like this are meant to be wrappers for using native APIs to begin with. Those APIs will have their expectations on where resources need to reside, specifications on file formats, resolution etc. Taken to a general level, I would say those problems you described aren't necessarily specific to Flutter or this plugin, rather it's the nature of cross-platform development

I'm using positional optional parameters on purpose, it's optimal for 9.0, it doesn't require changes to legacy code.

Fair enough and that's fine. This is why I mentioned options

[MaikuB]

Closing as this PR has become stale with lack of updates. Feel free to re-open if you are able to revisit this