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

Adds "class objects as arguments" RFC #29

Conversation

mitchhentges
Copy link
Contributor

@mitchhentges mitchhentges commented Jul 19, 2019

This RFC doesn't have a specific implementation goal, Johan and I were discussing architecture on an API boundary and haven't reached a satisfying conclusion, so we opted to tap into the insight of more of the team 😄

We're requesting reviews from ideally at least Mihai and Aki because Mihai is closest to Releng mobile, and Aki is really familiar with the scriptworker ecosystem.
Rendered.

@escapewindow
Copy link
Contributor

This seems like a design decision, and I'm unconvinced we need to standardize this across releng. I agree types can be beneficial, yet I also believe that python's flexible typing is an asset. I don't think this is a big enough win to mandate the change unless the major stakeholders all agree it's the best path forward.

This also seems like it doesn't have anywhere near initial consensus, so an rfc is probably the wrong choice.

@mitchhentges
Copy link
Contributor Author

Hey, good points, I'm definitely not looking to standardize this across releng.
However, I needed a platform on which further discussion could occur and where inline comments could be made.
Emails don't allow code formatting, and Slack isn't great for inline comments and responses.

So, I'm using the RFC repo less because this is a releng-wide proposal for standardization, but more because Github provides one of the best discussion platforms, and our RFC repo was the most relevant place to have the discussion

@bhearsum
Copy link
Contributor

I don't want to pile on too hard, but I also don't think an RFC is the right place for this. Is there a reason this discussion doesn't make sense to stay in the mozapkpublisher repo? If a wider audience is wanted/needed perhaps a link to that could be sent around?

@mitchhentges
Copy link
Contributor Author

@mozbhearsum I agree that this isn't a good fit for an RFC, but where does it make more sense?
My needs for this kind of discussion are:

  • Individual discussion points should be possible to annotate inline (Github PRs do a great job of this)
  • Once the discussion is resolved, it doesn't litter mozapkpublisher with an obsolete file (so, it doesn't make sense to PR this into mozapkpublisher)
  • It should be possible to format the discussion to make it easier to understand (Github markdown does this well)
  • The discussion should be accessible to the wider team so additional reviewers can be brought in as necessary
  • The discussion wasn't resolvable with a Zoom meeting, so it needs to be possible to work on asynchronously over a larger time period (days)

Based on these constraints, releng-rfcs made the most sense, but it's still an awkward fit. Is there a better workflow for this kind of chat?

@bhearsum
Copy link
Contributor

If it doesn't fit within the context of an existing PR or Issue in another repo, maybe a Google Doc is a good option, and then it can be linked to from any relevant places?

@mitchhentges
Copy link
Contributor Author

Google docs don't allow as convenient formatting as Github markdown, but that does sound better overall. I'll keep that in mind for the next discussion.
Once this one is complete, I'll close the PR rather than landing

@escapewindow
Copy link
Contributor

+1, or a mozapkpublisher issue.

@mitchhentges
Copy link
Contributor Author

The downside of making it a mozapkpublisher issue is that we can have inline annotations, you need to do the whole "quote and respond" technique, which makes it hard to have different conversation threads

@escapewindow
Copy link
Contributor

Issue linking to a gist?

@mitchhentges
Copy link
Contributor Author

I don't believe that you can leave inline comments on a gist

@tomprince
Copy link
Contributor

While this discussion doesn't make sense as something that should land in this repository, it isn't obvious that it shouldn't be used for this kind of discussion (with the end result being the PR is closed), as long as it is clear that that is what is happening.

@tomprince
Copy link
Contributor

(Perhaps it would be better as a draft PR, to make landing blocked, as well).

@bhearsum
Copy link
Contributor

While this discussion doesn't make sense as something that should land in this repository, it isn't obvious that it shouldn't be used for this kind of discussion (with the end result being the PR is closed), as long as it is clear that that is what is happening.

I agree it isn't obvious or a given, but it's a very notable change to use this repo as a mailing list or discussion board instead of its original purpose. The downside to doing so is that it goes from something with relatively low traffic and high importance, to something high traffic that people will necessarily stop treating with the same importance. If the main problem we're trying to solve is "good UX for arbitrary discussions" I would suggest that we create a repo for that purpose, use Google Docs, or consider using whatever IRC ends up getting replaced with, assuming it addresses the need.

@mitchhentges
Copy link
Contributor Author

Mihai responded here, closing

@mitchhentges mitchhentges deleted the class-objects-as-arguments branch July 24, 2019 22:37
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