insightsengineering / teal

Exploratory Web Apps for Analyzing Clinical Trial Data
https://insightsengineering.github.io/teal/
Other
176 stars 35 forks source link

do we need list wraped around module? #1235

Closed kartikeyakirar closed 4 months ago

kartikeyakirar commented 4 months ago

image

based on code if it's a single module, wrap it around alist. I feel it's older code and should be wrapped around teal_modules instead. But I am a bit unsure if its for added flexibility or should removing the older code .

@donyunardi @gogonzo Can you confirm this?

_Originally posted by @kartikeyakirar in https://github.com/insightsengineering/teal.connectors.ssh/pull/1#discussion_r1616079387_

donyunardi commented 4 months ago

That snippet of code was added with this PR. The idea is to allow flexibility when passing the object to the modules argument. With the update, users can wrap a single module with list() or modules(), or just pass the module object directly to the modules argument.

So the answer is no, users don't need list() wrapped around a single module. However, we allow a single module to be wrapped in list().

chlebowa commented 4 months ago

I recall, there was some discussion around this later on, before CRAN release, and @pawelru insisted that list remain a valid input.

kartikeyakirar commented 4 months ago

thanks @donyunardi and @chlebowa